HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Let's play Find that File

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fileplayletthatfind

Problem

Isn't it annoying when you can't find a file you recently created? Especially when you either have a very disorganized drive or a directory with lots of files.

Even if you don't care, isn't it nice to have a program be able to locate a file for you?

I did this because I have files that I can't seem to locate, but I know roughly where it is. The FileFinder class can:

  • Find your file



  • Copy it to somewhere else



The only feature-lack:

  • Files with the same name in a different directory are not handled well.



  • Only one of the files will be returned.



I will add that feature once I can think of a way to implement it.

```
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.StandardCopyOption;
import java.util.HashMap;
import java.util.Map;
import java.util.regex.Pattern;

public class FileFinder {

protected final File root;
protected Map files;

protected final Pattern matchingRegex;

public FileFinder(File root, String matchingRegex) {
if (root.isFile()) {
throw new IllegalArgumentException("root must be a directory.");
}
this.root = root;
this.files = getAllFiles(root);
this.matchingRegex = Pattern.compile(matchingRegex);
}

protected Map getAllFiles(File root) {
File[] files = root.listFiles();
Map result = new HashMap<>();
for (File file : files) {
if (file.isDirectory()) {
result.put(file.getName(), file);
result.putAll(getAllFiles(file));
} else if (matchingRegex.matcher(file.getName()).matches()) {
result.put(file.getName(), file);
}
}
return result;
}

public File getRoot() {
return root;
}

public File find(String fileName) throws FileNotFoundException {
File result = files.get(fileName);
if (result == null) {
throw new F

Solution

Bug

In the constructor, matchingRegex must be initialized before calling getAllFiles(root);, otherwise a NullPointerException is thrown if the root folder contains subfolders.

Design

Constructor

I agree with @holroy about the fact that there are too many things done in the constructor. Its job is too heavy, instead of just initialize some things. Moreover, it makes the FileFinder instance unusable for other parameters, e.g. to search in another folder or with another regex, one will need to instantiate a new FileFinder.

getAllFiles(File)

It should be named getAllMatchingFiles, because in the current implementation it returns only the files with names matching the regex and all the directories. By the way, it is not very clear why the directories are put there.

The implementation of this method is correct in general, but is coded entirely with before-Java-7 style. Indeed, java.nio would be a lot of help here.

First of all, there is FileVisitor interface. Using a SimpleFileVisitor prevents us from pre-calculating Map files. It also solves the problem of files with same names in different folders:

public Collection findFiles(String fileName) throws IOException {
  final Collection foundFiles = new ArrayList<>();
  Files.walkFileTree(this.root.toPath(), 
                     new SimpleFileVisitor() {
      @Override
      public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
                throws IOException {
        if (file.getFileName().toString().equals(fileName)) {
            foundFiles.add(file);
        }
        return FileVisitResult.CONTINUE;
      }
    });
  return foundFiles;
}


This implementation will return all the files having fileName. If you want to get them by regex, it's achieved with a little change in the if condition.

If we push a bit deeper and try to benefit from Java 8, it becomes really concise:

public Collection findFiles8(String fileName) throws IOException {
  return Files.find(this.root.toPath(), 
                    Integer.MAX_VALUE, // NB! should be more reasonable :)
                    (file, attrs) -> {
         return Files.isRegularFile(file) 
           && file.getFileName().toString().equals(fileName);
    }).collect(Collectors.toList());
}


In fact, Files.find(args) can do almost all the job of FileFinder...

copy(args)

I think that this method should be defined elsewhere, because it does not correspond to the functionality of FileFinder.

It's a good idea to use Files.copy() inside it (and even better would be instead of it). Since this is a public method, the validation of the arguments should be done on non-nullability of file and dest. The existence of the files will be checked in Files.copy(), with the respective exceptions thrown.

The target Path object can be created easier with dest.toPath().resolve(file.getName()), no need to instantiate a File.

Code Snippets

public Collection<Path> findFiles(String fileName) throws IOException {
  final Collection<Path> foundFiles = new ArrayList<>();
  Files.walkFileTree(this.root.toPath(), 
                     new SimpleFileVisitor<Path>() {
      @Override
      public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
                throws IOException {
        if (file.getFileName().toString().equals(fileName)) {
            foundFiles.add(file);
        }
        return FileVisitResult.CONTINUE;
      }
    });
  return foundFiles;
}
public Collection<Path> findFiles8(String fileName) throws IOException {
  return Files.find(this.root.toPath(), 
                    Integer.MAX_VALUE, // NB! should be more reasonable :)
                    (file, attrs) -> {
         return Files.isRegularFile(file) 
           && file.getFileName().toString().equals(fileName);
    }).collect(Collectors.toList());
}

Context

StackExchange Code Review Q#113448, answer score: 6

Revisions (0)

No revisions yet.