4
\$\begingroup\$

I have recently gotten into Java in a Computer Science class at my high school and I am trying to learn more than just the basics I have learned in school. Yesterday, I designed a very simple text editor I named Aqua that is written with Swing. For some reason, my computer drags a little bit when I run these methods. Is it because I have a crappy computer or did I write something wrong? Thanks!

private void save(String content, String name) throws IOException{
    System.out.println(dir.toString());
    if(firstRun<1){
        dirCreation();
        firstRun++;
    }
    try{
            String savedText = content;
            System.out.println(savedText);
            File newTextFile = new File(newDir.toString() + File + name + ".aqua");
            System.out.println(newDir.toString() + File.seperator + name + ".aqua");
            if (!newTextFile.exists()) {
                System.out.println("Created new File");
                newTextFile.createNewFile();
            }
        try (FileWriter fw = new FileWriter(newTextFile)) {
            fw.write(savedText);
        }

    }
    catch(IOException x){
                System.err.format("IOException: %s%n", x);
    }
}
private void load(String name) throws FileNotFoundException, IOException{
    if(firstRun<1){
        dirCreation();
        i++;
    }

    File loadingFile = new File(newDir + "\\" + name + ".aqua");
    Scanner scan = new Scanner(loadingFile);
    StringBuilder loadedText = new StringBuilder("");
    while (scan.hasNextLine()) {
            loadedText.append(scan.nextLine() + "\n");
            //Is this correct usage of StringBuilder?
    }
    jTextArea1.setText(out);        
    }
\$\endgroup\$
3
  • 3
    \$\begingroup\$ Use jvisualvm to find what actually causes the slowness. Also do not use + or concat in a loop to build up a string. Use StringBuilder instead. \$\endgroup\$ Commented Jun 10, 2013 at 7:01
  • \$\begingroup\$ @abuzittingillifirca internally the JVM uses a StringBuilder to perform string concatenation, so it does not really matter \$\endgroup\$ Commented Jun 10, 2013 at 7:08
  • 4
    \$\begingroup\$ @fge The JVM uses internally a StringBuffer but it is converted back to String after each line. When using StringBuffer dedicated you only have one conversion at the end of all operations. But StringBuilder has the avantage that it is not synchronied. \$\endgroup\$ Commented Jun 10, 2013 at 8:16

2 Answers 2

Reset to default
6
\$\begingroup\$

There are formatting and whitespace issues in your code, fix them! You're also switching between declaring variables and initializing them in the next line and initializing them in the same line, be consistent!

if(i<1){

That hurts! What is i? Where does it come from? Why do you check against less then one? Keep your variable names always clear, f.e. numberOfLinesInFile is way better then i in my opinion.

new File(newDir.toString() + "\\" + name + ".aqua");

This assumes that it is run on a Windows machine (or anything else that uses a backslash as path separator...which I hope no one else does). That will lead lead to problems on *nix machines, but I know that / works on any platform, you could also go full length and use File.separator.

Or you use the constructor-overload that is provided for exactly these means:

new File(newDir.toString(), name + ".aqua"):

String out = "";

That's not a great name for that variable, text or content would be better suited.

out+=line + "\n";

This has one problem, concatenating strings with the + is slow like whatever-place-of-ternal-pain-you-believe-in-if-any. Use a StringBuilder, that will speed things up considerable.

jTextArea1.setText(out);

Again, not a good name.

\$\endgroup\$
5
  • 1
    \$\begingroup\$ StringBuilder is used by string concatenation internally! Your comment on slowness of string concatenation was true on pre 1.5 days, but not anymore \$\endgroup\$ Commented Jun 10, 2013 at 7:47
  • \$\begingroup\$ @fge: Do you have a link to that? \$\endgroup\$ Commented Jun 10, 2013 at 7:51
  • \$\begingroup\$ Yep, here \$\endgroup\$ Commented Jun 10, 2013 at 7:53
  • 6
    \$\begingroup\$ @fge: Thank you. But that only handles building of one line, not the overall text. A simple test application shows a difference: + took 3 seconds vs StringBuilder 0. If I set it to 100,000 iterations, it's at...uhm, + never did finish within 10 minutes, StringBuilder takes 16ms... \$\endgroup\$ Commented Jun 10, 2013 at 8:14
  • \$\begingroup\$ Okay, thanks you for the response, I will update the code now. \$\endgroup\$ Commented Jun 10, 2013 at 13:37
6
\$\begingroup\$

First and most important: you are using Java 1.7 (as the try-with-resource statement proves), so do yourself a favor: use Files! The File API should have been marked as deprecated ever since this new API was here.

In most situations, the Files class itself is all you need:

Path dir = Paths.get("/path/to/directory");
Path file = dir.resolve("nameOfFile");
// Create a directory, including parents
Files.createDirectories(dir);
// Write the contents of a string to a file
Files.copy(new ByteArrayInputStream(text.getBytes("UTF-8")), file);

Drop File. Real fast. Among the many, many problems it has, look at this line in your code:

// DOES NOT THROW AN EXCEPTION IF IT FAILS!
newTextFile.createNewFile();
\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the reply, this is really helping me a lot. Also, in another question, do i need and how do i use different threads? \$\endgroup\$ Commented Jun 10, 2013 at 13:39
  • \$\begingroup\$ Given what you do, I don't see the need for several threads. \$\endgroup\$ Commented Jun 10, 2013 at 13:50

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.