5
\$\begingroup\$

Here is my code. I simulated a stack rather than used some regular recursive algorithm because I needed to keep track of two variables: the path to the biggest file and its size. Besides I wrote a few tests.

My solution on Github.

DirectoryIterator.java

import java.io.File;
import java.util.ArrayDeque;
import java.util.Deque;

public class DirectoryIterator {

    private DirectoryIterator() {
        throw new AssertionError();
    }

    public static String getLargestFilePath(File initPath) {
        if (initPath == null) {
            throw new IllegalArgumentException("The init path cannot be null");
        }
        long maxFileLength = 0;
        String maxFilePath = null;
        Deque<File> stack = new ArrayDeque<>();
        stack.push(initPath);
        while (!stack.isEmpty()) {
            File current = stack.pop();
            if (current.isDirectory()) {
                File[] files = current.listFiles();
                if (files != null) {
                    for (File file : files) {
                        stack.push(file);
                    }
                }
            } else if (current.length() > maxFileLength) {
                maxFileLength = current.length();
                maxFilePath = current.getAbsolutePath();
            }
        }
        return maxFilePath;
    }

}

DirectoryIteratorTest.java

import org.junit.Assert;
import org.junit.Test;

import java.io.File;

public class DirectoryIteratorTest {

    private String mTestDir = "C:\\Users\\Maksim\\Downloads\\Test" + File.separatorChar;

    @Test
    public void singleFileIsInit() {
        File initPath = new File(mTestDir + "Android_Accelerometer.png");
        Assert.assertEquals(initPath.getAbsolutePath(), DirectoryIterator.getLargestFilePath(initPath));
    }

    @Test
    public void onlyDirs() {
        File initPath = new File(mTestDir + "only_dirs");
        Assert.assertEquals(null, DirectoryIterator.getLargestFilePath(initPath));
    }

    @Test(expected = IllegalArgumentException.class)
    public void nullPath() {
        Assert.assertEquals(null, DirectoryIterator.getLargestFilePath(null));
    }

    @Test
    public void emptyPath() {
        File initPath = new File("");
        Assert.assertEquals(null, DirectoryIterator.getLargestFilePath(initPath));
    }

    @Test
    public void manyFiles() {
        File initPath = new File("C:\\Users\\Maksim\\Google Drive");
        Assert.assertEquals("C:\\Users\\Maksim\\Google Drive\\Career\\TODO\\AnDevCon Boston 2015.zip", DirectoryIterator.getLargestFilePath(initPath));
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Are you on java 8? \$\endgroup\$ – Vogel612 Jan 25 '16 at 7:59
  • 5
    \$\begingroup\$ Even on Java 7, you can do this a lot easier with Files.walkFileTree and overriding SimpleFileVisitor<Path>. \$\endgroup\$ – Tunaki Jan 25 '16 at 9:02
5
\$\begingroup\$

I find it weird that your function accepts a File but returns a String. I would choose to return a File.

According to the JavaDoc for listFiles(),

If this abstract pathname does not denote a directory, then this method returns null. Otherwise an array of File objects is returned, one for each file or directory in the directory.

Based on the documented behaviour, I would say that your if (current.isDirectory()) and if (files != null) checks are redundant, and I would eliminate the current.isDirectory() check.

It might be a good idea to do something about the possible SecurityException if you don't have permission to list a directory.

The throw new AssertionError() in the private constructor is an interesting idea that I haven't seen before. Perhaps it would be better written as assert false;.

\$\endgroup\$
0
\$\begingroup\$

You mention that u haven't used recursion because you needed to keep track of two variables. You can still use recursion, perhaps like this :

public class DirectoryIterator {

    private DirectoryIterator() {
        throw new AssertionError();
    }

    //an instance of this class to keep track of both the variables
    private static class FileAttributes {
        String filePath = null;
        long fileLength = 0;
    }

    //public method called
    public static String maxFilePath(File initPath) {
        if (initPath == null) {
            throw new IllegalArgumentException("The init path cannot be null");
        }
        FileAttributes fileAttributes = new FileAttributes();
        maxFilePath(initPath, fileAttributes);
        return fileAttributes.filePath;
    }

    //recursive method private, file attributes shared across calls
    private static void maxFilePath(File current, FileAttributes maxFileAttributes) {
        if (current.isDirectory()) {
            for (File file : current.listFiles()) {
                maxFilePath(file, maxFileAttributes);
            }
        } else if (current.length() > maxFileAttributes.fileLength) {
            maxFileAttributes.fileLength = current.length();
            maxFileAttributes.filePath = current.getAbsolutePath();
        }
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ You have presented an alternative solution, but haven't reviewed the code. Please explain your reasoning (how your solution works and how it improves upon the original) so that the author can learn from your thought process. \$\endgroup\$ – Vogel612 Jan 25 '16 at 7:59

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.