5
\$\begingroup\$

I am writing a brute-force method to search for a word. Please review my code and tell me how I can improve it.

public class Searcher {

  private static String filepath = null;
  private static final String defaultPath = "//sample_text";

  private Searcher() {
    this.filepath = defaultPath;
  }

  private Searcher(String dir) {
    if (dir.isEmpty()) {
      this.filepath = defaultPath;
    } else {
      this.filepath = dir;
    }
  }

  public static void main(String[] args) throws IOException {
    Searcher search = new Searcher();

    String folderToSearch = search.filepath;

    File folder = new File(folderToSearch);
    Set<File> list = new HashSet<File>();
    search.getFiles(folder, list);

    String toSearch = "The";
    for (File file : list) {
     BruteForceSearch bSerch = new BruteForceSearch(file);
     bSerch.search(toSearch);
    }
  }

  private void getFiles(File folder, Set<File> list) {

    folder.setReadOnly();
    File[] files = folder.listFiles();
    for (int j = 0; j < files.length; j++) {
      list.add(files[j]);
      if (files[j].isDirectory())
        getFiles(files[j], list);
    }
  }

}

public class BruteForceSearch implements WordSearch{

  private final String fileName;
  private Map<String, Integer> result =  new HashMap<String, Integer>();
  private File file;
  private int count =0;


  public BruteForceSearch(File file) throws IOException {
    this.file =  file;
    this.fileName = file.getName();
    this.count = 0;
  }

  private void searchBruteForce(String toSearch) throws IOException {

      FileInputStream fstream = new FileInputStream(file);
      BufferedReader in = new BufferedReader(new InputStreamReader(fstream));
      String readLine = "";
      while ((readLine = in.readLine()) != null) {
        String[] words = readLine.split("\\W");
        for (String text : words) {
          if (text.equalsIgnoreCase(toSearch)) {
            count++;
          }
        }
      }
      in.close();
  }

  public String getFile() {
    return fileName;
  }

  public int getCount(String word) {
    return count;
  }

  public void search(String toSearch) throws IOException {
    searchBruteForce(toSearch);
    if (getCount(toSearch) != 0) {
        System.out.println(getFile() + " - " + getCount(toSearch) + " matches ");   
    } 
  }
}
public interface WordSearch {

  String getFile();
  int getCount(String term);
}

Here the interface is used for other search methods as well (like using regex, using index based). Brute-force is one such implementation.

\$\endgroup\$
  • \$\begingroup\$ In response to your comment on your deleted question about processing multiple files, yes, it'll be nice if the result can be in the form of a line, n-th word when searching for a specific word. I suggested only lines since that seems to be the basis of your questions here so far, about doing word searches in files. BTW, any particular reason why you chose to delete that? It's still quite on-topic for this site, and I don't see any particular reason for you to do so. \$\endgroup\$ – h.j.k. Jan 31 '16 at 3:06
5
\$\begingroup\$

Semantic of methods

The implementation or the signature of the method "int getCount(String term);" does not make sense. Either you delegate the parameter "term" to the search algorithm with a stateless implementation or you omit it and have a stateful implementation. In the last case your "term" should be given to the constructor. I prefer a stateless implementation and all further comments are related to that.

Programming using interfaces

You are not using the interface "WordSearch". The assigment

BruteForceSearch bSerch = new BruteForceSearch(file);

be to the abstraction:

WordSearch search = new BruteForceSearch(file);

Avoid passing working list references

The code

Set<File> list = new HashSet<File>();
search.getFiles(folder, list);

should be refactored to:

Set<File> list = search.getFiles(folder);

The "List" should be instantiated within the method "getFiles(...)".

Make directory recursion part of WordSearch through template pattern

Currently you are collecting all files recursively within the main-method. Make the algorithm part of the WordSearch objects. To do so, refactor the WordSearch-Interface to an abstract class and implement the directory recursion as a Template Method.

public abstract class WordSearch {

    ...

    public int getCount(String term) {
        File folder = ...;
        return getCountInFolder(folder);
    }

    /**
     * Template method pattern
     */
    protected abstract int getCountInFile(File file, String term);

    private int getCountInFolder(File folder, String term) {
        int count = 0;
        File[] files = folder.listFiles();
        for (int j = 0; j < files.length; j++) {
            if (files[j].isDirectory()) {
                count = count + getCountInFolder(files[j], term);
            } else {
                count = count + getCountInFile(files[j], term);
            }
        }
        return count;
    }

    ...

}

Introduce strategy pattern

Associate a WordSearch-Object within "Searcher" and delegate to it. This is called Strategy pattern.

public class Searcher {

    private WordSearch wordSearch;

    public Searcher(String dir, WordSearch wordSearch) {
        this.wordSearch = wordSearch;
        ...
    }

    public int getCount(String term) {
        return this.wordSearch.getCount(term);
    }

    public void setWordSearch(WordSearch wordSearch) {
        this.wordSearch = wordSearch;
    }

    ...

}

The WordSearch-Object may be set through the constructor and changed by a setter.

\$\endgroup\$
1
\$\begingroup\$

Have you read about the Pattern class?

String string = "The quick brown fox jumps over the other brown fox"
int occurence = 0;
Pattern pattern = Pattern.compile("brown fox");
Matcher matcher = pattern.matcher(string);
while (matcher.find()) {
    occurence++;
}

This is more flexible as it can also match whole expressions and use regex as input, too. It's also a faster implementation.

\$\endgroup\$
  • 1
    \$\begingroup\$ I am using the pattern for RegEx based search. Like I mentioned, I am having 3 searches : 1. Brutforce 2.RegEx 3.Index based. To see the performance of the searches. \$\endgroup\$ – Sarah Jan 30 '16 at 19:02

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.