4
\$\begingroup\$

This question came from a Java tutorial online:

Write an example that counts the number of times a particular character, such as e, appears in a file.

I was just wondering if there is any way to improve this particular solution.

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;

public class CountLetter{
    public static void main(String[] args) {
        Path file = Paths.get("/Users/justin/Desktop/Codes Netbean/JavaRandom/xanadu.txt");
        CountLetter cl1 = new CountLetter(file, 'e');
        System.out.println(cl1.count());
    }

    private Path file;
    private char lookFor;

    CountLetter(Path file, char lookFor){
        this.file = file;
        this.lookFor = lookFor;
    }

    private int count(){
        int count = 0;
        try(BufferedReader br = new BufferedReader(new InputStreamReader(Files.newInputStream(file)))){
            String line = null;
            while((line = br.readLine()) != null){
                for(int i = 0; i < line.length(); i++){
                    if(line.charAt(i) == lookFor){
                        count++;
                    }
                }
            }
        } catch (IOException x){
            System.err.println(x);
        }
        return count;
    }
}
\$\endgroup\$

3 Answers 3

1
\$\begingroup\$

Although there already is a selected answer ill put my answer opinion in perspective.

Essentially what is recommended in every situation to do is to split your code into functions where each function does its own part. I've split up your code into a readFile() function which reads the entire .txt file and converts it to a string and essentially does all the heavy lifting, and a count() function which counts the char you want it to count.

The readFile() function :

private String readFile() IOException {

    String everything;

    try (BufferedReader br = new BufferedReader(file01)) {
        StringBuilder sb = new StringBuilder();
        String line = br.readLine();

        while (line != null) {
            sb.append(line);
            sb.append(System.lineSeparator());
            line = br.readLine();

        }
        everything = sb.toString();
    }
    return everything;
}

And count() function :

private int count(String line) {
    int count = 0;

    for (int i = 0; i < line.length(); i++) {
        if (line.charAt(i) == lookFor) {
            count++;
        }
    }

    return count;
}

The point of this separation is to make the code more flexible for example, what if you had to read four files and search how much a certain character would occur in all of them, this way you can instance them all and count easily. Flexibility is key IMO.

Also I've modified the whole code to be more flexible:

import java.io.*;

public class CountLetter {

private final char lookFor;
private FileReader file01 = new FileReader("/Users/justin/Desktop/Codes Netbean/JavaRandom/xanadu.txt");

CountLetter(FileReader file, char lookFor) throws IOException {
    this.file01 = file;
    this.lookFor = lookFor;

}

private String readFile() throws IOException {

    String everything;

    try (BufferedReader br = new BufferedReader(file01)) {
        StringBuilder sb = new StringBuilder();
        String line = br.readLine();

        while (line != null) {
            sb.append(line);
            sb.append(System.lineSeparator());
            line = br.readLine();

        }
        everything = sb.toString();
    }
    return everything;
}

private int count(String line) {
    int count = 0;

    for (int i = 0; i < line.length(); i++) {
        if (line.charAt(i) == lookFor) {
            count++;
        }
    }

    return count;
}

public static void main(String[] args) throws IOException {
    FileReader file01 = new FileReader("/Users/justin/Desktop/Codes Netbean/JavaRandom/xanadu.txt");
    CountLetter cl1 = new CountLetter(file01, 'a');

    System.out.println(cl1.count(cl1.readFile()));
}

}

Considering, the fact that i'm still a semi-beginner in the whole programming world go easy on me if i had made any terrible mistakes, just trying to help out.

\$\endgroup\$
4
\$\begingroup\$

If you are using java 8 you can use streams.

Also IMO for this kind of functionality a static method would me more appropriate.

 public static long countCharacter(Path path, char lookFor) throws IOException {

     try (Stream<String> stream = Files.lines(path)) {

         return stream.flatMapToInt(String::chars)
                      .filter(c -> c == (int) lookFor)
                      .count();
     }
 }
\$\endgroup\$
1
\$\begingroup\$

I recommend you pass the path of a file into count(String filePath) method. The reason is we can use a single instance of CountLetter to count different files. Secondly, we shouldn't ask api consumers Path here as their code will be decoupled to our implementation of opening files. By doing that, you could switch to another way of reading files as you please and these changes doesn't break the clients' code.

public class CountLetter {
    public iny count(String filePath) {
        // same implementation
    }
}

public static void main() {
   CountLetter countLetter = new CountLetter();
   countLetter.count("file1");
   countLetter.count("file2");
   // ....
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.