8
\$\begingroup\$

I'm trying to improve my code and i want to know if this is the best way to write this Switch or if there are other methods.

for(String word : words) {
        switch(word.toLowerCase()) {
        case "a":
            a++;
            break;
        case "the":
            the++;
            break;
        case "bird":
            bird++;
            break;
        case "animal":
            animal++;
            break;
        case "is":
            is++;
            break;
        }
    }

Here is the complete code if you need it.

public static void main(String[] args) throws IOException {
ArrayList<String> words = new ArrayList<>();
int a = 0, the = 0, bird = 0, animal = 0, is = 0;

    for(String line : Files.readAllLines(Paths.get("C:\\Users\\n\\Desktop\\Text.txt"))) {
        line.split("\\s+");
        line.replaceAll("[!?.,]", "");
        for(String word : line.split("\\s+")) {
            words.add(word);
        }
    }

    for(String word : words) {
        switch(word.toLowerCase()) {
        case "a":
            a++;
            break;
        case "the":
            the++;
            break;
        case "bird":
            bird++;
            break;
        case "animal":
            animal++;
            break;
        case "is":
            is++;
            break;
        }
    }
System.out.println(a + " " + the + " " + bird + " " + animal + " " + is);
}
\$\endgroup\$

migrated from stackoverflow.com Dec 12 '14 at 18:07

This question came from our site for professional and enthusiast programmers.

  • \$\begingroup\$ Is it your goal to count how many of a particular word you have found in the text file? \$\endgroup\$ – rayryeng Dec 12 '14 at 18:06
  • 1
    \$\begingroup\$ A better approach would be to have a HashMap<String,Integer>. \$\endgroup\$ – chiastic-security Dec 12 '14 at 18:07
  • \$\begingroup\$ Yes, that's my goal. \$\endgroup\$ – vincentes Dec 12 '14 at 18:09
  • \$\begingroup\$ line.replaceAll("[!?.,]", ""); will not affect original string, but will create new one with replaced (or in this case removed) characters so if you want to update line you should store this new string like line = line.replaceAll("[!?.,]", "");. \$\endgroup\$ – Pshemo Dec 12 '14 at 18:12
  • \$\begingroup\$ Weird. I printed out all the words and none had symbols. \$\endgroup\$ – vincentes Dec 12 '14 at 18:18
6
\$\begingroup\$

Here's a better, more efficient and compact way:

    String path = "C:/Users/n/Desktop/Text.txt";

    List<String> targetList = Arrays.asList("a", "the", "bird", "animal", "is");
    Map<String, Integer> counts = new HashMap<>(targetList.size());
    for (String word : targetList) {
        counts.put(word, 0);
    }

    for (String line : Files.readAllLines(Paths.get(path))) {
        for (String word : line.replaceAll("[!?.,]", "").toLowerCase().split("\\s+")) {
            Integer count = counts.get(word);
            if (count != null) {
                counts.put(word, count + 1);
            }
        }
    }

    System.out.print(counts.get(targetList.get(0)));
    for (int i = 1; i < targetList.size(); ++i) {
        String word = targetList.get(i);
        System.out.print(" " + counts.get(word));
    }
    System.out.println();

The improvements and corrections:

  • It's good to define constants like the path high up in a file where they are easy to change and easy to change, without having to read into the details of the code
  • It's simpler to write paths with forward slashes
  • Use interface type like List when defining a list instead of implementation type like ArrayList
  • Since it seems you're only interested in a specific set of words:
    • I put them in a list for ordering
    • ... then initialized the map of counts to all 0 values
  • Instead of building a list of words, it's more efficient to do the counting at the same time as you read the words. This will save you both storage and processing time
  • When you do line.replaceAll("[!?.,]", ""), the operation is not performed on line, as strings in Java are immutable. The result with the characters removed is returned
  • The same goes for a line.split("\\s+") statement you had. If you don't save the result of the operation, then it's completely pointless
\$\endgroup\$
10
\$\begingroup\$

You can use a Map :

Map<String,Integer> freq = new HashMap<String,Integer>();
for(String word : words) {
    String w = word.toLowerCase();
    if (freq.containsKey(w)) {
        freq.put(w,freq.get(w)+1);
    } else {
        freq.put(w,1);
    }
}
\$\endgroup\$
  • \$\begingroup\$ I'm confused how this works. I have read the java docs and I'm still confused. \$\endgroup\$ – vincentes Dec 12 '14 at 18:15
  • 1
    \$\begingroup\$ @Vincent Map stores key-value pairs. Key needs to be unique, values not. We can use map which will be storing word<String>-count<Integer> pair. If you are putting word into map first time (if map doesn't have it) you can place pair word,1, but if map already have such word you need to put into map this word with new value, which will be equal to old value + 1 (you want to increase counter by getting old counter and adding 1 to it). \$\endgroup\$ – Pshemo Dec 12 '14 at 18:21
  • 2
    \$\begingroup\$ @Vincent Instead of having a different variable that counts the occurences of each word, you use a Map, where the key is the word and the value is the number of occurences. If the current word is not in the Map, you add it to the Map with a initial count of 1. If it's already in the Map, you increment the count by 1. In the end, freq.get("bird") would return the value that the bird variable holds in your implementation. \$\endgroup\$ – Eran Dec 12 '14 at 18:21
  • 4
    \$\begingroup\$ would you please insert the explanation into the answer? Thank you. \$\endgroup\$ – Malachi Dec 12 '14 at 19:07
  • 1
    \$\begingroup\$ Also, if you're using guava you can use a MultiMap \$\endgroup\$ – mjgpy3 Dec 15 '14 at 14:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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