patternjavaMinor
Search word in multiple files
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
```
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
be to the abstraction:
Avoid passing working list references
The code
should be refactored to:
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.
Introduce strategy pattern
Associate a WordSearch-Object within "Searcher" and delegate to it. This is called Strategy pattern.
The WordSearch-Object may be set through the constructor and changed by a setter.
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.