Finding empty folders

Question

I wrote a program that finds empty folders and subfolders in a directory. It's the first program I've ever written in Java, and I'd like some critique. I feel like I'm not utilizing classes in a very coherent manner. Also, I think I'm naming variables poorly, because as I write my program I find myself constantly having to rename methods/classes/variables to clarify their meaning.

Part 1: Under the Hood

package emptyfolders;
import java.io.File;
import java.util.*;
public class EmptyFolders {
 public static void main(String[] args) {
 }
 private List seekEmpty(String dirname, List emptyFoldersTemp) {
 // This method goes through all folders that are directly contained in a
 // directory, and appends them to a list if they are empty. Used
 // recursively for directories containing multiple folders
 // Get the appropriate directory from user input
 File dir = new File(dirname);
 // Populate an array with files/folders in the directory
 File[] folderContents = dir.listFiles();
 // Iterate through every file/folder
 for (File currentContent : folderContents) {
 // Disregard files, acquire folders
 if (currentContent.isDirectory()) {
 // If this folder is empty, add it to the list
 if (currentContent.listFiles().length == 0) {
 emptyFoldersTemp.add(currentContent);
 // If not, run this method on the folder
 } else if (currentContent.listFiles().length >= 1) {
 seekEmpty(currentContent.toString(),emptyFoldersTemp);
 } 
 }
 }
 // Return a list containing all empty folders currently found
 return emptyFoldersTemp;
 }
 List listLooper(String folder) {
 // An outer program that helps seekEmpty with its recursion, instantiate
 EmptyFolders directory = new EmptyFolders();
 // Create a temporary list that holds empty folders
 List emptyFoldersTemp = new ArrayList();
 //Run seekEmpty 
 List emptyFolders = directory.seekEmpty(folder, emptyFoldersTemp);
 //Return the list of empty subfolders
 return emptyFolders;
 }
}

listLooper feels redundant, especially the part where it causes the program to return the List Object through two methods. All the stuff there was originally in the main method, but I needed the method to take a single String (for reasons you will see later, it grabs a directory from a UI). Should I get rid of it, and if so, how?

Part 2: The UI

private void listButtonActionPerformed(java.awt.event.ActionEvent evt) { 
 // When the "List" button is pressed, pull the directory from the
 // text field, and search that directory for empty folders and subfolders
 //Get directory from directory field and double all the backslashes so 
 //for functional purposes, note that since backslashes are literals,
 //they are doubled in the "replace" method
 String targetDirectory = directoryField.getText().replace("\\","\\\\");
 //Instantiate an instance of the EmptyFolder finder
 EmptyFolders emptyFolderFinder = new EmptyFolders();
 //Run the empty folder finder recursive method (separate file)
 List emptyFolders = emptyFolderFinder.listLooper(targetDirectory);
 //Clear the text area, if it contains text
 if (! emptyFoldersArea.getText().equals("")) {
 emptyFoldersArea.setText("");
 }
 for (Object emptyFolder : emptyFolders) {
 //Append all empty folders found in the target directory
 emptyFoldersArea.append(emptyFolder.toString() + "\n");
 }
} 
private void browseButtonActionPerformed(java.awt.event.ActionEvent evt) { 
 // When the "Browse..." button is pressed, the UI prompts the user to
 // select the directory he or she wants to check for empty folders, and
 // the program calls a method to do so
 //Instantiate an instance of the file browser
 JFileChooser fileBrowser = new JFileChooser();
 //Rename title, specify that we are only concerned with folders
 fileBrowser.setDialogTitle("Select target directory");
 fileBrowser.setFileSelectionMode(JFileChooser.DIRECTORIES_ONLY);
 //Return the directory on affirmation and write it to UI
 int returnValue = fileBrowser.showDialog(this, null);
 if (returnValue == JFileChooser.APPROVE_OPTION) {
 String targetDirectory = fileBrowser.getSelectedFile().toString();
 directoryField.setText(targetDirectory);
 } else {
 }
}

I built a Swing UI using Netbeans, so I'm cutting that code out. What I care about are two buttons in the UI. One is a browse button that the user uses to pick a directory, and the other is a button that runs the program. The list of empty folders is displayed in an empty area. I don't know how to better integrate this with my program, I feel like it sticks out in a way that doesn't flow with the rest of my program.

I'm taking college programming classes when I come back from the break, so this was my way of exposing myself to Java early, so any advice before I get started is appreciated (fingers crossed that CS is right for me).

• Clockwork-Muse: You're currently in danger of infinite recursion if the folder structure is cyclical (eg, a shortcut for Desktop is placed on the Desktop...). I doubt Windows or other OSs have any of these by default, but you can't guarantee that some random program (or person) has at some point (actually, Win7 has an option to show a link to the User directory on the desktop, which would do this). The common strategy is to retain a list of all items visited (linked lists have similar problems). Also, you probably want Set, as there's no reason to store a folder more than once.

Answer 1

There are a few small items, and a couple of larger ones that concern me in your code...

• You have an empty main method under your hood.... it serves no purpose, so remove it ;-)

• you are not using the generic typing for your List. In this case, your code should look like:

  List<File> emptyFolders = emptyFolderFinder.listLooper(targetDirectory);
  

  and the listLooper method should return List<File>.

• in your listLooper method you declare your List<File> and then pass it in to the seelEmpty method. This method returns the exact same List instance back, so it should not have to return a value at all. The input List<File> is an 'accumulator' that you add things to. There is no reason to return it as well.

• seekEmpty takes a String dirname as a parameter. This should not be a String, but a File. You already have the file when you call the method, so you convert the file to String, and then immediately convert it back. just keep it as a File the whole time.

• in seekEmpty (again) you have the code which checks for the number of files in a Directory:

  if (currentContent.listFiles().length == 0) { .... } else if (currentContent.listFiles().length>= 1) { ... }

  the else-if makes no sense because, if the length is not == 0 then it can only be >= 1. It should be a simple } else { and not have an else if at all.

OK, now for some of the bigger things...

You ask whether you should get rid of the listLooper method. The answer is probably no. You are setting up a recursive call chain with the seekEmpty method. You often need an 'entry' to a recursive method, and the listLooper is this method. The method will pre-and-post process the data required by, and created by the recursion.

Right, you want to find directories which have no files....

What I don't like about your code is that it calls 'listFiles()` twice. We can fix that with/in the recursion....

A method like the following will serve you better, I think:

/** This method goes through all folders that are directly contained in a
 * directory, and appends them to a list if they are empty. Used
 * recursively for directories containing multiple folders
 *
 * @param dir is the directory to check for content
 * @param emptyFolders is he List we add empty folders to.
 */
private void seekEmpty(File dir, List<File> emptyFolders) {
 // Populate an array with files/folders in the directory
 File[] folderContents = dir.listFiles();
 if (folderContents.length == 0) {
 // we are empty, add us.
 emptyFolders.add(dir);
 }
 // Iterate through every file/folder
 for (File content : folderContents) {
 // Disregard files, acquire folders
 if (content.isDirectory()) {
 // check if this folder is empty
 seekEmpty(content, emptyFolders);
 }
 }
}

This significantly simplifies the recursion.

• itsmichaelwang: Thanks! You've helped realize the importance of coding with increased emphasis on efficiency and, IMO, maintainability. Are there any resources on good coding practices that you can recommend?
• rolfl: @Zapurdead Joshua Bloch ... Effective Java, then Java Concurrency In Practice ... can't go wrong.