Random song playing app improvements

Question

I have a substantial music collection, which will not load entirely in any music program.

So I wrote this quick program, to play a random song in a subfolder of my music directory.

I'm just wondering if anyone can improve on this code? It works pretty well, but the subdirectories are not all equal with respect to size, so it neglects some directories. My random mod might also be a problem. This is just to have a useful shuffle.

import java.io.File;
import java.io.IOException;

public class Pick {
public static void main(String args[]) throws IOException {

    File f = new File("C:\\MUSIC");
    File random = null;
    try {
        random = goDeeper(f);
    } catch (Exception e) {
    }
    while (random == null) {
        try {
            random = goDeeper(f);
        } catch (Exception e) {
        }
    }

    System.out.println(random.getAbsolutePath());
    Process myProcess = Runtime.getRuntime().exec(
            "\"C:\\Program Files (x86)\\foobar2000\\foobar2000.exe\"   \""
                    + random.getAbsolutePath() + "\"  ");

}

public static File goDeeper(File f) {
    File[] children = f.listFiles();

    int rand = (int) ((Math.random() * 123456) % children.length);
    File random = children[rand];
    if (random.isDirectory()) {
        return goDeeper(new File(f.getAbsolutePath() + "/"
                + random.getName()));
    } else if (random.isFile() && random.getName().endsWith("mp3"))
        return random.getAbsoluteFile();

    return null;
}

}

Answer 1

Don't repeat yourself

The try-random-catch block appears two times in your code. The easiest way to fix this would be to delete the first occurrence. You don't need it, as the while check will be true on the first loop, thus executing that block.

Handling Exceptions

There are a lot of ways to handle an exception, but just silently ignoring it is not one of them. It will make it very hard to find bugs.

Alternatives are:

• just let the exception bubble up all the way
• log the exception to a file (for detailed information use getStackTrace)
• print the exception to console (best use printStackTrace, not getMessage)
• handle the exception

It is also important to not catch a blanket Exception, but the specific exceptions that may be thrown. With your code, a reader doesn't really know what might be thrown (and thus has no idea at all what might have gone wrong).

Random file selection

Your selection process isn't really random.

1. Let's say you have structure like this:

   /
   /dir
   /song.mp3
   /dir/nonMp3File
   /dir/anotherSong.mp3
   

Now, we enter goDeeper, and randomly select dir. On the next iteration, we randomly select nonMp3File. Now, null is returned and we restart at /. That means if you have a lot of non-mp3 files in you music folder (cover art, etc), the deeper nested the file, the less likely it will be that it will be selected, because the goDeeper returned null first.

2. Even if you fix 1., your program will still favor songs that are less deeply nested inside subdirectories.

This might not be a bad thing (it depends on the structure of your music directory, and how you want to select) though.

3. the hardcoded 123456 lets me suspect that you are not really randomly selecting among the children. Something like rand = new Random().nextInt(children.length + 1); might be better.

Infinite loop

If you don't have any MP3 files, the program will loop forever. If the ratio of MP3 to non-MP3 is bad, the program might loop for a long time before playing a song.

Naming

• random might be randomFileName (both in main and goDeeper).
• f might be baseDirectory (for the variable as well as the parameter).
• goDeeper might be getRandomFileName.

Misc

• Use curly braces, even for one-line statements to avoid bugs and to increase readability.
• Remove unused variables: you are never using myProcess, so just remove it (not the call to exec, just the variable declaration).

Answer 2

About this:

return goDeeper(new File(f.getAbsolutePath() + "/"
            + random.getName()));

A more prefered way of constructing a new File object is:

new File(f, random.getName());

But wait a minute... That is exactly the same file as random in the first place.

new File(random);

But as you're just creating a new file object for the same file, you don't really need to create a new file object at all, just use it directly.

return goDeeper(random);