patternjavaMinor
Counting occurrences of a word in a file
Viewed 0 times
filewordcountingoccurrences
Problem
This gives me the output of how many times the word is in the file. (I will eventually want to enhance the program to output on what line of the file the word is.)
```
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.io.BufferedReader;
import java.io.FileReader;
import java.io.*;
public class CountLineWordsDuplicateWords {
public FileReader fr = null;
public BufferedReader br =null;
public String [] stringArray;
public int counLine = 0;
public int arrayLength ;
public String s="";
public String stringLine="";
public String filename ="";
public String wordname ="";
public CountLineWordsDuplicateWords(){
try{
Scanner scan = new Scanner(System.in);
System.out.println("Please enter the filename: ");
filename = scan.nextLine();
Scanner scan2 = new Scanner(System.in);
System.out.println("Please enter a word: ");
wordname = scan.nextLine();
fr = new FileReader(filename);
br = new BufferedReader(fr);
while((s = br.readLine()) != null){
stringLine = stringLine + s;
//System.out.println(s);
stringLine = stringLine + " ";
counLine ++;
}
//System.out.println("Contents of file: " + stringLine);
stringArray = stringLine.split(" ");
arrayLength = stringArray.length;
//System.out.println("The total number of words in the text file is "+arrayLength);
/Duplicate String count code /
for (int i = 0; i < arrayLength; i++) {
int c = 1 ;
for (int j = i+1; j < arrayLength; j++) {
if(stringArray[i].equalsIgnoreCase(stringArray[j])){
c++;
for (int j2 = j; j2 < arrayLength; j2++) {
stringArray[j2] = stringArray[j2+1];
arrayLength = arrayLength - 1;
}
```
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.io.BufferedReader;
import java.io.FileReader;
import java.io.*;
public class CountLineWordsDuplicateWords {
public FileReader fr = null;
public BufferedReader br =null;
public String [] stringArray;
public int counLine = 0;
public int arrayLength ;
public String s="";
public String stringLine="";
public String filename ="";
public String wordname ="";
public CountLineWordsDuplicateWords(){
try{
Scanner scan = new Scanner(System.in);
System.out.println("Please enter the filename: ");
filename = scan.nextLine();
Scanner scan2 = new Scanner(System.in);
System.out.println("Please enter a word: ");
wordname = scan.nextLine();
fr = new FileReader(filename);
br = new BufferedReader(fr);
while((s = br.readLine()) != null){
stringLine = stringLine + s;
//System.out.println(s);
stringLine = stringLine + " ";
counLine ++;
}
//System.out.println("Contents of file: " + stringLine);
stringArray = stringLine.split(" ");
arrayLength = stringArray.length;
//System.out.println("The total number of words in the text file is "+arrayLength);
/Duplicate String count code /
for (int i = 0; i < arrayLength; i++) {
int c = 1 ;
for (int j = i+1; j < arrayLength; j++) {
if(stringArray[i].equalsIgnoreCase(stringArray[j])){
c++;
for (int j2 = j; j2 < arrayLength; j2++) {
stringArray[j2] = stringArray[j2+1];
arrayLength = arrayLength - 1;
}
Solution
Your code is way more complicated that it needs to be. Let's look at it in detail
You're creating 2
Writing this generally means that something is wrong. You're opening a stream to a resource but, in case there is an exception, they won't be closed: the
part of the code won't be reached. This creates a resource leak. Starting with Java 7, you can use a try-with-resources statement so that every opened resource is properly closed, regarless of the outcome (expection or not).
Then,
concatenates Strings in a loop, which is a bad practice: Strings are immutable and every time you use
You're splitting in an array all the Strings you concatenated before, meaning you did all that work for nothing: you could have just created an array and populate it in the first place.
Now this is what the main loop is doing:
So to put it another way, this is traversing the array as many times as there are elements, which can be a lot. Now imagine if the array is very long, this can take a very long time.
You don't even need to do this: you can just traverse the array a single time and determine if the current element matches your searched word.
This could be a proposed implementation: it uses the built-in
Sample code:
Scanner scan = new Scanner(System.in);
System.out.println("Please enter the filename: ");
filename = scan.nextLine();
Scanner scan2 = new Scanner(System.in);
System.out.println("Please enter a word: ");
wordname = scan.nextLine();You're creating 2
Scanner object when you just need one to read both the filename and the word to search. You can reuse the Scanner you already have.fr = new FileReader(filename);
br = new BufferedReader(fr);Writing this generally means that something is wrong. You're opening a stream to a resource but, in case there is an exception, they won't be closed: the
fr.close();
br.close();part of the code won't be reached. This creates a resource leak. Starting with Java 7, you can use a try-with-resources statement so that every opened resource is properly closed, regarless of the outcome (expection or not).
Then,
while((s = br.readLine()) != null){
stringLine = stringLine + s;
//System.out.println(s);
stringLine = stringLine + " ";
counLine ++;
}concatenates Strings in a loop, which is a bad practice: Strings are immutable and every time you use
+ a new object is created each time. Instead, what you want to use is a StringBuilder. But in this case...stringArray = stringLine.split(" ");
arrayLength = stringArray.length;You're splitting in an array all the Strings you concatenated before, meaning you did all that work for nothing: you could have just created an array and populate it in the first place.
Now this is what the main loop is doing:
- Start at the first word;
- Look up a word equal, ignore case, to this word;
- If one is found, move all the words after that to the left;
- Continue with the second element, until the end of the array
So to put it another way, this is traversing the array as many times as there are elements, which can be a lot. Now imagine if the array is very long, this can take a very long time.
You don't even need to do this: you can just traverse the array a single time and determine if the current element matches your searched word.
This could be a proposed implementation: it uses the built-in
LineNumberReader (to match your enhancement), wraps it inside a try-with-resources. Then each line read is split around a space and the words are compared against the word to search.Sample code:
public static void main(String[] args) throws IOException {
Scanner scan = new Scanner(System.in);
System.out.println("Please enter the filename: ");
String filename = scan.nextLine();
System.out.println("Please enter a word: ");
String wordname = scan.nextLine();
int count = 0;
try (LineNumberReader r = new LineNumberReader(new FileReader(filename))) {
String line;
while ((line = r.readLine()) != null) {
for (String element : line.split(" ")) {
if (element.equalsIgnoreCase(wordname)) {
count++;
System.out.println("Word found at line " + r.getLineNumber());
}
}
}
}
System.out.println("The word " + wordname + " appears " + count + " times.");
}Code Snippets
Scanner scan = new Scanner(System.in);
System.out.println("Please enter the filename: ");
filename = scan.nextLine();
Scanner scan2 = new Scanner(System.in);
System.out.println("Please enter a word: ");
wordname = scan.nextLine();fr = new FileReader(filename);
br = new BufferedReader(fr);fr.close();
br.close();while((s = br.readLine()) != null){
stringLine = stringLine + s;
//System.out.println(s);
stringLine = stringLine + " ";
counLine ++;
}stringArray = stringLine.split(" ");
arrayLength = stringArray.length;Context
StackExchange Code Review Q#122263, answer score: 6
Revisions (0)
No revisions yet.