CODEWITHSUNDEEP
javabufferedwriter
KeirDavis
KeirDavis

Reputation: 665

Write-to-file code doesn't write

Well, I am trying to write a line of information to log in a text file (.txt) but this is the part where the code fails to write. Everything else works, except this but doesn't give any errors.

public void writeConfig(File config, Boolean append, String errored){
    try {
        Writer output;
        if (config != null){
            output = new BufferedWriter(new FileWriter(config));
        } else {
            output = new BufferedWriter(new FileWriter(er));
        }
        if (append == true){
            output.append(errored);
        } else {
            output.write(errored);
        }
    } catch (Exception e){
        try {
            loadErrorLog(error, true, "Failed to write to Boom's Log.\n");
        } catch (Exception e1){
            log.info("Major Malfunction #686 : Tell Maker immediatly.");
        }
    }
}

Upvotes: 0

Views: 234

Answers (1)

Jon Skeet
Jon Skeet

Reputation: 1500225

You're not closing the writer, which means all the data is just staying in the buffer.

You should close it in a finally block.

Additionally:

  • your use of the append parameter is distinctly dodgy - you should almost certainly be passing it to the constructor of the FileWriter (or FileOutputStream). I don't think append in Writer does what you think it does.
  • Try to avoid comparing with true - just if (append) would have been clearer

  • Using the conditional operator could make your FileWriter code cleaner, especially if you used it just for the file:

    File file = config == null ? er : config;
Writer writer = new BufferedWriter(new FileWriter(file));
  • I would avoid using FileWriter in the first place, as it always uses the platform default encoding. Use a FileOutputStream wrapped in an OutputStreamWriter instead, specifying the encoding explicitly (e.g. UTF-8)
  • Avoid catching Exception in most places; here it would be cleaner to just catch IOException.
  • It looks like your loadErrorLog method should probably be doing that logging on failure, otherwise I suspect you'll be writing that block of code every time you call it.
  • Do you really want to continue if you fail to write the config? Is it definitely this method which should handle the exception? I'd potentially remove the catch block entirely (leaving just a try/finally) and let the IOException bubble up the stack

Upvotes: 4

Related Questions