4
\$\begingroup\$

I am making a Swing application for light local database management and I have the button Run XAMPP.

When that button is pressed this code is executed:

private void jRunXAMPPButtonActionPerformed(java.awt.event.ActionEvent evt)                                                
    {                                                    
        String[] paths = {"C:\\xampp\\xampp-control.exe",
            "C:\\Program Files\\xampp\\xampp-control.exe",
            "C:\\Program Files (x86)\\xampp\\xampp-control.exe"};

        for (String path : paths) {
            final File file = new File(path);

            if (file.exists()) {
                try {
                    Process p = Runtime.getRuntime().exec(file.getAbsolutePath());
                } catch (IOException e) {
                    continue;
                }
                return;
            }
        }
        Helper.printErrln("xampp-control.exe was not found or is corrupt!"); // Prints error in red
    }

I basically have the three possible paths that xampp could be at and I go through them and check if the file exists and can be executed. If not then it prints the error that the file could not be found or is corrupt.

Do you have any tips for my code? Any better way of doing this and I'm especially concerned that my error handling is not that great.

\$\endgroup\$

2 Answers 2

5
\$\begingroup\$
  • Give your method a proper name. These auto-generated names are pure terror!
  • You can import ActionEvent directly, no need to reference it with the entire package name.
  • Your formatting is unusual at best, I'd call it bad. I know this is, for some reason, the preferred way to set braces in the Linux kernel code, but this is Java, not C.
  • You are using hard-coded file paths. Absolute no-go. What if the user named his main partition D or installed xampp somewhere else – or is using Mac or Linux?
  • Why are you storing the result in Process p if you never use that variable?
  • " // Prints error in red" is a comment that is both completely useless (why do I need to know this right then and there?) and will rot faster than you can imagine.
\$\endgroup\$
1
  • \$\begingroup\$ Because I am creating a simple software for mostly personal use (for now) I am not interested in either Linux, Mac or the possibility of the path being something completely different. As for your other points I believe you are right except the braces, I like keeping braces that way since I work with both Java and C++ and it keeps my code generally consistent between the two. \$\endgroup\$
    – Aki K
    May 9, 2014 at 13:17
3
\$\begingroup\$

I agree 100% with @ingo-burk's answer. Here are a few points on top of that.

You could load the paths from a .properties file quite easily, for example given a properties file on the classpath:

# config.properties
paths = C:/xampp/xampp-control.exe|C:/Program Files/xampp/xampp-control.exe|C:/Program Files (x86)/xampp/xampp-control.exe

and this Java code:

private static final String PROPERTIES_FILENAME = "player.properties";
private static final String PATHS_PROPERTY = "paths";
private static final String ITEM_SEPARATOR = "|";

private String[] getPaths(String filename, String propName) throws IOException {
    InputStream input = this.getClass().getClassLoader().getResourceAsStream(filename);
    if (input == null) {
        return new String[0];
    }
    Properties prop = new Properties();
    prop.load(input);
    input.close();
    return prop.getProperty(propName, "").split(ITEM_SEPARATOR);
}

private String[] getPaths(String filename) throws IOException {
    return getPaths(filename, PATHS_PROPERTY);
}

String[] getPaths() throws IOException {
    return getPaths(PROPERTIES_FILENAME, PATHS_PROPERTY);
}

Notice that I changed all the path separators from \\ to /. Java figures out the correct path separator in the operating system, and it's simpler to type this way.

Btw, if the executable is always called xampp-control.exe then perhaps you can put that name in its own property, and remove the duplication from paths.

\$\endgroup\$
1
  • \$\begingroup\$ Hmmm I already have a method for checking if a property file is corrupted or missing and for getting a specific property from a property file so I guess I can use my code and save some time. All in all I like this approach. Also yes the file will always be called xampp-control.exe \$\endgroup\$
    – Aki K
    May 9, 2014 at 13:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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