-1

My task is to show a tree of all directories/files of a PC drive, I have a class DirectoryNode that extends DefaultMutableTreeNode with File field directoryPath. I build nodes recursively:

public void buildDirectoryTree(){
    if(!directoryPath.isDirectory()){
            return;
    }

    for(File f : directoryPath.listFiles()){
        if(f.isHidden() || !f.exists()) continue;
        DirectoryNode newChild = new DirectoryNode(f);
        add(newChild);
        newChild.buildDirectoryTree();
    }
}

It works fine for concrete directories, but when I try to use it for whole drive, or some large directories, JTree with this node does not show up at all I think it encounters a problem with specific directories. I've add exists and is Hidden checks to skip this problem roots, but it didn't help.

In addition, exists, isHidden and isDirectory return false for some of my valid directories directories (I am using Windows 10).

Andrew Thompson
  • 168,117
  • 40
  • 217
  • 433
TheSmokingGnu
  • 302
  • 2
  • 6
  • 15
  • 1
    First, I'd use the nio2 `Path` API rather than `File`, as `File` doesn't like symbolic links. I'd also be careful with scanning a whole drive and make sure you're using a "lazy" loading approach, so you only load what you need to display – MadProgrammer Apr 02 '17 at 11:15
  • 1
    1) See also the [File Browser GUI](http://codereview.stackexchange.com/q/4446/7784). It builds each part of the tree on user selection of a directory, the main reason for that is that it takes a loooooong time to build a tree of an entire (typical) drive. 2) For better help sooner, post a [MCVE] or [Short, Self Contained, Correct Example](http://www.sscce.org/). – Andrew Thompson Apr 02 '17 at 12:27
  • 1
    Also consider this [example](http://stackoverflow.com/a/34224804/230513) that uses a `FileTreeModel`. – trashgod Apr 02 '17 at 16:10

1 Answers1

1

File.listFiles() is one of those ancient methods that violates Java's convention/good practice to never return null from a method that returns an array. So you have to check for null.

From the docs:

An array of abstract pathnames denoting the files and directories in the directory denoted by this abstract pathname. The array will be empty if the directory is empty. Returns null if this abstract pathname does not denote a directory, or if an I/O error occurs.

I have changed your code to make it a little safer. If it's called from the EDT, you might want to add some log message or the like instead of throwing the exception into nirvana.

public void buildDirectoryTree() throws IOException {
    if (!directoryPath.isDirectory()) {
            return;
    }
    final File[] files = directoryPath.listFiles();
    if (files != null) {
        for (File f : files) {
            if (f.isHidden() || !f.exists()) continue;
            DirectoryNode newChild = new DirectoryNode(f);
            add(newChild);
            newChild.buildDirectoryTree();
        }
    } else {
        throw new IOException("Failed to list files for " + directoryPath);
    }
}

As others have pointed out, there are more modern APIs and they have been introduced for good reasons. I recommend to read up on NIO2 and the Path APIs for better solutions.

Hendrik
  • 5,085
  • 24
  • 56