6
\$\begingroup\$

I was practicing Java by building some of the challenges on this website: https://codingchallenges.fyi/challenges/challenge-wc/

I've built a word counter clone and I wanted to know if my solution is efficient. I've tested the code against a couple of input files and compared it with the output from wc command. And it seems to be working fine. I would appreciate any feedback on this!

package WordCount;

import java.io.BufferedReader;
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.io.Reader;
import java.io.StringReader;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Scanner;

public class WordCountJava {
    /**
     * Options
     * c = bytes in input
     * l = lines in input
     * w = words in input
     * m = characters in input
     */
    private boolean c = false, l = false, w = false, m = false;
    private long lineCount = 0, wordCount = 0, byteCount = 0;
    private final List<String> fileNames;

    public WordCountJava() {
        fileNames = new ArrayList<>();
    }

    private void processArguments(String[] args) {
        for (String arg : args) {
            arg = arg.toLowerCase(Locale.ROOT);

            if (arg.charAt(0) == '-') {
                for (int index = 1; index < arg.length(); index++) {
                    char option = arg.charAt(index);

                    switch (option) {
                        case 'c' -> {
                            c = true;
                            m = false; // Cancel out prior usage of m
                        }
                        case 'l' -> l = true;
                        case 'w' -> w = true;
                        case 'm' -> {
                            m = true;
                            c = false; // Cancel out prior usage of c
                        }
                        default -> {
                            System.out.println("Invalid option: \"" + option + "\"");
                            System.exit(0);
                        }
                    }
                }
            } else {
                fileNames.add(arg);
            }
        }
    }

    private void printToStdOut(String fileName) {
        StringBuilder builder = new StringBuilder();

        if (l) {
            builder.append(lineCount).append(" ");
        }

        if (w) {
            builder.append(wordCount).append(" ");
        }

        if (c || m) {
            builder.append(byteCount).append(" ");
        }

        builder.append(fileName);

        System.out.println(builder);
    }

    private void countFileSize(String filePath) {
        File file = new File(filePath);
        byteCount = file.length();
    }

    private void countCharacters(BufferedReader br) throws IOException {
        int character = br.read();

        while (character != -1) {
            byteCount++;
            character = br.read();
        }
    }

    /**
     * Called when option `m` is requested along with other options
     */
    private void countCharactersAndOthers(BufferedReader br) throws IOException {
        int charInt = br.read();
        char character = (char) charInt;
        char previous = '0';

        while (charInt != -1) {
            while (charInt != -1 && Character.isWhitespace(character)) {
                if (previous != '\r' && (character == '\n' || character == '\r')) {
                    lineCount++;
                }

                byteCount++;
                charInt = br.read();
                previous = character;
                character = (char) charInt;
            }

            if (charInt != -1) {
                wordCount++;

                while (charInt != -1 && !Character.isWhitespace(character)) {
                    byteCount++;
                    charInt = br.read();
                    previous = character;
                    character = (char) charInt;
                }
            }
        }
    }

    /**
     * Called when only `l` is passed
     */
    private void countLines(BufferedReader br) throws IOException {
        String line = br.readLine();

        while (line != null) {
            lineCount++;
            line = br.readLine();
        }
    }

    /**
     * Called when only `lw` is passed
     */
    private void countWordsWithLines(BufferedReader br) throws IOException {
        String line = br.readLine();

        while (line != null) {
            int index = 0;
            int lineLength = line.length();

            while (index < lineLength) {
                while (index < lineLength && Character.isWhitespace(line.charAt(index))) {
                    index++;
                }

                if (index < lineLength) {
                    wordCount++;

                    while (index < lineLength && !Character.isWhitespace(line.charAt(index))) {
                        index++;
                    }
                }
            }

            lineCount++;
            line = br.readLine();
        }
    }

    private void process(String fileName, BufferedReader br) throws IOException {
        if (l || w || m) {
            if (m) {
                if (l || w) {
                    countCharactersAndOthers(br);
                } else {
                    countCharacters(br);
                }
            } else if (l && !w) {
                countLines(br);
            } else {
                countWordsWithLines(br);
            }
        }

        printToStdOut(fileName);
    }

    private void processFile(String fileName) {
        String pwd = Path.of("").toAbsolutePath().toString();
        String filePath = pwd + "/" + fileName;

        if (c) {
            countFileSize(filePath);
        }

        try (BufferedReader br = new BufferedReader(new FileReader(filePath))) {
            process(fileName, br);
        } catch (IOException e) {
            throw new RuntimeException(e);
        }
    }

    private void processStdInput(String input) {
        if (c) {
            byteCount = input.getBytes().length;
        }

        try (Reader str = new StringReader(input);
             BufferedReader br = new BufferedReader(str)) {
            process("", br);
        } catch (IOException e) {
            throw new RuntimeException(e);
        }
    }

    private void start(String[] args) {
        processArguments(args);

        // If no options are passed, count all of them - c, l and w
        if (!(c || l || w || m)) {
            c = l = w = true;
        }

        if (!fileNames.isEmpty()) {
            for (String fileName : fileNames) {
                processFile(fileName);
            }
        } else {
            Scanner scanner = new Scanner(System.in);
            StringBuilder input = new StringBuilder();
            String line = "";

            while (scanner.hasNextLine()) {
                line = scanner.nextLine();
                input.append(line).append('\n');
            }

            processStdInput(input.toString());
        }
    }

    public static void main(String[] args) throws IOException {
        WordCountJava wordCountJava = new WordCountJava();
        wordCountJava.start(args);
    }
}

\$\endgroup\$
4
  • 1
    \$\begingroup\$ Good thing that the class contains the name of the language it's written in. Who'd otherwise know which one it was. scnr \$\endgroup\$ Apr 14 at 10:03
  • \$\begingroup\$ Lol. I don't know why I named it like that. \$\endgroup\$
    – Reacher
    Apr 14 at 18:56
  • \$\begingroup\$ Please do not edit the question after answers have been provided as to not invalidate said answers. \$\endgroup\$ Apr 14 at 21:05
  • \$\begingroup\$ Sorry about that. Will keep that in mind. \$\endgroup\$
    – Reacher
    Apr 14 at 22:13

2 Answers 2

6
\$\begingroup\$

Expand c, l, w and m to proper variable names.

In typical unix programs, arguments are never case-folded; I would drop your toLowerCase() and just assume case sensitivity.

It's less safe to "cancel out prior usage of m", and more safe to enforce that those arguments are mutually exclusive. Otherwise, the user may be surprised.

WordCountJava is redundant - anything written in Java is Java, so call it something like WordCounter.

This is not a very well-modelled object-oriented class structure. Once you have some args, you should be able to construct a new counter instance whose arguments are final and don't vary. start should not accept args.

\$\endgroup\$
0
6
\$\begingroup\$

Byte count and character count are not the same thing. Let's consider the following test on my personal computer which defaults to UTF-8 encoding:

$ echo "öäå" | wc -mcwl
       1       1       4       7

That's one line, one word, 4 characters and 7 bytes. And that's correct since I am using a Unix variant that uses one character for newlines.

When reading stdin, your implementation does not seem to take into account the fact that line break on Windows machines is 2 characters.

Another big problem is that your code reads everything from stdin into memory twice. First it constructs the StringBuilder that holds everything that was read from stdin and then it calls toString which creates a new copy of the string, hoping that there is enough RAM for you to step out of the method and give the garbage collector the permission to free up the StringBuilder.

You've put all your code into one class. This makes it hard to test. You should study single responsibility principle. Split your code into classes so that each class has one responsibility. From the top of my head, some candidates could be a class that is responsible for parsing the command line parameters, a class that is responsible for reading the input (stdin vs multiple files), a class that collects the counts and a class that produces the output.

A nice solution to implementing the byte counting while also taking advantage of Java built in character encoding support from InputStreamReader would be to use something like a CountingInputStream. And of course, you must remember that BufferedReader.readLine() will again read the whole stdin into memory (twice, because it too uses StringBuilder) in case there are no line breaks in the input. So you probably have to implement your own Unix/Windows compliant line break detection. And remember that in EBCDIC the line break character is 0x15. :D

An old programming widsdom to finish this review:

No program can be considered production ready until a character encoding bug has been found and fixed.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for such a detailed feedback! I was kind of lazy while implementing the stdin part and half-assed it, but I'll definitely try to incorporate these suggestions. \$\endgroup\$
    – Reacher
    Apr 15 at 8:45

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.