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

Search word in multiple files

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

Problem

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 list = new HashSet();
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 list) {

folder.setReadOnly();
File[] files = folder.listFiles();
for (int j = 0; j result = new HashMap();
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() + " - " + get

Solution

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 list = new HashSet();
search.getFiles(folder, list);


should be refactored to:

Set 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.

Code Snippets

BruteForceSearch bSerch = new BruteForceSearch(file);
WordSearch search = new BruteForceSearch(file);
Set<File> list = new HashSet<File>();
search.getFiles(folder, list);
Set<File> list = search.getFiles(folder);
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;
    }

    ...

}

Context

StackExchange Code Review Q#118370, answer score: 5

Revisions (0)

No revisions yet.