1
\$\begingroup\$

Can this method be modified to be more efficient?

public void saveFile(File file, TextArea textArea) {
    if (file != null) {
        String filePath = file.getAbsolutePath();
        try (BufferedWriter writer = Files.newBufferedWriter(Paths.get(filePath))) {
            writer.write(textArea.getText());
        } catch (IOException e) {
            System.out.println("savefile exception");
        }
    }
}
\$\endgroup\$
6
\$\begingroup\$

Some suggestion:

  1. A "save to file" method without a valid file is useless. If the file is null your method is a no-op without a warning for the user.
  2. Code reuse: your code works with TextArea only. A better solution would be a String argument instead of TextArea.
  3. Method name: "saveFile" is too generic
  4. Exception handling: you should catch the exception only if you know how to handle it or if you enrich the exception with new information before rethrow it

My version:

public void writeStringToFile(File file, String text) throws IOException {
    if (file == null)
        throw new IllegalArgumentException("file cannot be null");

    if (text == null)
        throw new IllegalArgumentException("text cannot be null");

    String filePath = file.getAbsolutePath();

    try (BufferedWriter writer = Files.newBufferedWriter(Paths.get(filePath))) {
        writer.write(text);
    }
}

But... DON'T REINVENT THE WHEEL!

A better solution is the Apache FileUtils class, see writeStringToFile method.

ps: English is not my mother tongue

\$\endgroup\$
  • \$\begingroup\$ Don't buy a new wheel when you already have one: what about Files.write in the various signatures? Still, best answer so far (+1) \$\endgroup\$ – mtj May 13 '17 at 12:43
  • \$\begingroup\$ I'll implement your suggestions.... \$\endgroup\$ – Pakbert May 15 '17 at 13:55
2
\$\begingroup\$

It seems that you go roundabout to create buffered writer, as new BufferedWriter(new FileWriter(file)); would suffice. In case of exception, it would make more sense to write to System.err stream as opposed to System.out. Also, depending on the context, I would rethrow exception, and catch in place where I am calling this method, so e.g. if I call saveFile method inside save method, I can show a message box saying save failed inside catch clause.

\$\endgroup\$
  • \$\begingroup\$ Au contraire: Files.newBufferedWriter should be the normal method today to create a buffered writer, that's why it has been added to the base library in Java 7. Please take this from the discussion to your own book of recipies. \$\endgroup\$ – mtj May 13 '17 at 12:41
  • \$\begingroup\$ Don't talk nonsense, both of these are equally normal to use. When you have a File already, using FileWriter makes a code more readable. I should share a page about not making sarcastic remarks from my recipe book one day. \$\endgroup\$ – Coderino Javarino May 13 '17 at 13:55
  • \$\begingroup\$ I think it always depends on the approach the programmer is taking. Newbie here, all comments are helpful to me. \$\endgroup\$ – Pakbert May 15 '17 at 14:01
2
\$\begingroup\$

To get the Path use File.toPath() instead of getting the absolute path as a string and passing it to the Path constructor.

As opposed to @coderino I like the use of Files.newBufferedWriter. It makes the code clearer. However you should specify the charset, otherwise the file may not be portable between systems as by default it uses the system default charset which depends on locale.

\$\endgroup\$
2
\$\begingroup\$

This is the modified method based on the suggestion given with the additional charset for user convenience. If this is still modifiable to be more efficient, please be patient, I'm a newbie.

public void writeToFile(File file, String text, Charset charset) throws IOException {

    if (file == null) {
        throw new IllegalArgumentException("Illegal argument, file cannot be null.");
    }
    if (text == null) {
        throw new IllegalArgumentException("Illegal argument, text cannot be null.");
    }

    try (BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(new FileOutputStream(file.getPath()), charset))) {
        writer.write(text);
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.