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
    \$\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\$ – abuzittin gillifirca Jun 10 '13 at 7:01
  • \$\begingroup\$ @abuzittingillifirca internally the JVM uses a StringBuilder to perform string concatenation, so it does not really matter \$\endgroup\$ – fge Jun 10 '13 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\$ – Uwe Plonus Jun 10 '13 at 8:16
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\$
  • 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\$ – fge Jun 10 '13 at 7:47
  • \$\begingroup\$ @fge: Do you have a link to that? \$\endgroup\$ – Bobby Jun 10 '13 at 7:51
  • \$\begingroup\$ Yep, here \$\endgroup\$ – fge Jun 10 '13 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\$ – Bobby Jun 10 '13 at 8:14
  • \$\begingroup\$ Okay, thanks you for the response, I will update the code now. \$\endgroup\$ – bigfoot675 Jun 10 '13 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\$
  • \$\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\$ – bigfoot675 Jun 10 '13 at 13:39
  • \$\begingroup\$ Given what you do, I don't see the need for several threads. \$\endgroup\$ – fge Jun 10 '13 at 13:50

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.