Java Word Frequency Counter

Question

This is a simple word frequency counter in Java. I am just wondering if I can improve or compact this code better. You choose a file to be counted with a JFileChooser, and the results get outputted into a file. The biggest struggle I had was sorting the HashMap by values. I got it to work but I feel like it should be easier to sort. I'm also wondering if I should break up my code in main into more methods. I saw a Python script of this that does the same thing but in half the lines of code. Is that just Java or does my code need touching up?

import java.io.BufferedWriter;
import java.io.File;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;

import javax.swing.JFileChooser;

public class FrequencyCounter {

    public static void main(String[] args) throws IOException {
        JFileChooser fc = new JFileChooser();
        File file = null;
        fc.setCurrentDirectory(new File(System.getProperty("user.dir")));
        if (fc.showOpenDialog(null) == JFileChooser.APPROVE_OPTION) {
            file = fc.getSelectedFile();
        }
        FileReader is = new FileReader(file);
        char[] scriptBytes = new char[(int) file.length()];
        is.read(scriptBytes);
        is.close();
        String script = new String(scriptBytes);
        String[] words = script.split("[ \\r?\\n\\-]");
        List<String> wordList = new ArrayList<>(Arrays.asList(words));
        wordList = cleaner(wordList);
        HashMap<String, Integer> counter = new HashMap<>();
        for (String word : wordList) {
            if (counter.containsKey(word))
                counter.put(word, counter.get(word) + 1);
            else
                counter.put(word, 1);
        }
        counter = entriesSortedByValues(counter);
        BufferedWriter writer = new BufferedWriter(new FileWriter(new File(file.getAbsolutePath().replace(file.getName(), "") + "result_" + file.getName())));
        for (Entry<String, Integer> ent : counter.entrySet()) {
            writer.write(ent.getKey() + " " + ent.getValue());
            writer.newLine();
        }
        writer.close();
    }

    private static List<String> cleaner(List<String> words) {
        for (int i = 0; i < words.size(); i++) {
            words.set(i, words.get(i).replaceAll("[1234567—890!@#$%^&*()_+|\\-=~`{}\\[\\]:;\"<>,.?/]","").toLowerCase());
        }
        words.removeAll(Arrays.asList(""));
        return words;
    }

    private static <K,V extends Comparable<? super V>> 
        HashMap<K, V> entriesSortedByValues(Map<K,V> map) {

        List<Entry<K,V>> sortedEntries = new ArrayList<Entry<K,V>>(map.entrySet());

        Collections.sort(sortedEntries, 
            new Comparator<Entry<K,V>>() {
                @Override
                public int compare(Entry<K,V> e1, Entry<K,V> e2) {
                    return e2.getValue().compareTo(e1.getValue());
                }
            }
        );
        HashMap<K,V> result = new LinkedHashMap<K,V>();
        for (Entry<K,V> ent : sortedEntries)
            result.put(ent.getKey(), ent.getValue());

        return result;
    }

}

Java is a verbose language, expect solutions written in it to be longer than in any dynamic language. — yuri
– yuri, Commented May 28, 2017 at 6:55
Does the Python script have a GUI for choosing files? If not, there's your answer. (I could do a far less efficient version of this in about 3 lines in Java (excluding boilerplate), utilising Java 8 streams) — Tamoghna Chowdhury
– Tamoghna Chowdhury, Commented May 28, 2017 at 8:43
Normally, you'll want methods outside the main class, they can the be referenced as static methods, also since your methods are in the main class, they don't have to be static — EastXWest
– EastXWest, Commented May 30, 2017 at 6:45

kraskevich · Accepted Answer · 2017-05-28 19:03:57Z

It's normal for Java programs to be longer. I wouldn't care about the number of lines per se.
The main method does too much stuff:
- It asks the user to choose the file
- It reads the content of the file
- It counts the number of occurrences
- It prints them to another file
One method should be responsible for one thing. There're at least 4 different (very different) things here.

In fact, I believe that the first thing (and probably the second and the last thing, too) doesn't belong in the FrequencyCounter class. Why? It makes sense for the counter to count occurrences and do just it. It'll make it more reusable. Think what happens if you want to change the output format, the way the user choose the file or just use it as part of your different project.
I would redesign the FrequencyCounter it so that it has one method that takes a string and returns a map with different words and the number of their occurrences.
The reading/writing stuff and the user dialog belong to a different class. These are different concerns. So I'd create another class to handle file I/O and one more class to interact with a user as these things have nothing to do together. They can change independently and they don't need each other to work. They're separate entities.
You don't handle the case when the file is null (that is, when the user doesn't choose a file). A NullPointerException isn't very user-friendly.
This piece of code:
```
 if (counter.containsKey(word))
     counter.put(word, counter.get(word) + 1);
 else
     counter.put(word, 1);
```
can be simplified to counter.put(counter.getOrDefault(word, 0) + 1); unless you're using an older Java version.
If you plan to work with large files (really large, such that their content doesn't fit into main memory), you'll need to get rid of reading the entire file into a String. A more space-efficient solution would be to read it in chunks and process them separately (the words are normally short, so it can save a lot of memory). Sure, it makes the code more complicated (for instance, you'll need to carefully check when the word ends), so it's worth doing only if you plan to work with large amounts of data.
A method name is normally a verb. cleaner doesn't sound very good to me. I'd rename it to something like cleanWords. I'd also put cleaning one word into a separate method to make it more manageable. You can go even further and put all cleaning stuff into a separate class to increase the flexibility (it also makes sense from a design point of view: cleaning and counting are different responsibilities and they are pretty much independent).
The way you work with the output file is potentially dangerous: if such a file already exists, it'll be overwritten. I don't think that doing it silently is a good idea. There're a few different ways to deal with it: generating a different unique name, aborting the program or letting the user choose the output file. Either of them is better than replacing the content of the file silently.

I'm wondering whether the multiple lines of import statements need to be fixed or not?? In another word, should we use import java.util.*; ? — Angela P
– Angela P, Commented May 31, 2017 at 18:58
@AngelaPan There's nothing to fix here. Your way is shorter, but it also imports the stuff we don't use. Neither of them is "always right", but I prefer explicit imports. — kraskevich
– kraskevich, Commented May 31, 2017 at 19:33
counter.put(counter.getOrDefault(word, 0) + 1); needs to be corrected to counter.put(word, counter.getOrDefault(word, 0) + 1); — acarlstein
– acarlstein, Commented Feb 15, 2019 at 15:17

Tobias Walter · Accepted Answer · 2017-05-28 19:56:45Z

Picking up the suggestion of @Tamoghna Chowdhury I wrote a sample implementation using Java8 Streams.

Note that the suggestions of @kraskevich aren't considered in my implementation. You should only consider this as a proof of concept of what is possible (in particular reducing the line count) by using the appropriate API of Java.

public class FrequencyCounter {

    private static Map<String, Long> countFrequency(Stream<String> valueStream) {
    return valueStream
            .map(s -> s.split(" "))
            .flatMap(Arrays::stream)
            .map(FrequencyCounter::clean)               
            .filter(s -> !s.equals(""))
            .collect(Collectors.groupingBy(Function.identity(), Collectors.counting()));
    }

    public static void main(String[] args) throws IOException {
        JFileChooser fc = new JFileChooser();
        fc.setCurrentDirectory(new File(System.getProperty("user.dir")));
        if (fc.showOpenDialog(null) == JFileChooser.APPROVE_OPTION) {
            Map<String, Long> map = countFrequency(
                    Files.lines(Paths.get(fc.getSelectedFile().getAbsolutePath())));

            BufferedWriter writer = new BufferedWriter(new FileWriter(new File(fc.getSelectedFile().getAbsolutePath().replace(fc.getSelectedFile().getName(), "") + "result_" + fc.getSelectedFile().getName())));
            for (Entry<String, Long> ent : map.entrySet()) {
                writer.write(ent.getKey() + " " + ent.getValue() + "\n");
                writer.newLine();
            }
            writer.close();
        }
    }

    private static String clean(String word) {
        return word.replaceAll("[1234567—890!@#$%^&*()_+|\\-=~`{}\\[\\]:;\"<>,.?/]", "").toLowerCase(); 
    }
}

\$\begingroup\$ This only splits by line, not word \$\endgroup\$

gommb
– gommb

2017-05-28 18:52:00 +00:00
Commented May 28, 2017 at 18:52 — gommb
– gommb, Commented May 28, 2017 at 18:52
\$\begingroup\$ You are right. I made an edit adding the necessary split \$\endgroup\$

Tobias Walter
– Tobias Walter

2017-05-28 19:57:06 +00:00
Commented May 28, 2017 at 19:57 — Tobias Walter
– Tobias Walter, Commented May 28, 2017 at 19:57

Stack Exchange Network

Java Word Frequency Counter

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Java Word Frequency Counter

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions