Find repeated substrings of length 3 in a string The following method receives a string and finds all occurrences of repeated substrings with a length of three characters, including spaces. Is this a good approach?

public Map&lt;String, Integer&gt; findOccurences(String str) {
        Map&lt;String, Integer&gt; map = new HashMap&lt;String, Integer&gt;();
        int counter = 0;
        String sub; 
        int stringLen = str.length() - 2;
        for (int i = 0; i &lt; stringLen ; i++) {
            sub = str.substring(i, i + 3);
            if (map.containsKey(sub)) { 
                counter = map.get(sub); 
                counter++;
                map.put(sub, counter);
                counter = 0;
            } else {
                map.put(sub, 1); 
            }
        }
        return map;
    }




Input 


This This 



Output


 Th 1    &lt;&lt;&lt; includes space 
s T 1
his 2
Thi 2
is  1    &lt;&lt;&lt; includes space
It's quite fine. The logic is simple and clear.
There might be a more clever solution (I don't know),
but probably it will be less clear.

A few improvements would be good though:


counter is set to 0 at two places, both unnecessary, as the value is overwritten later
It's a good practice to declare variables in the smallest scope where they are used. For example counter and sub would be better to declare inside the loop.
stringLen is not a great name, because it's not really the "length" of anything. Even limit would be better
Instead of the magic number 3, and str.length() - 2, it would be better to make the target length a method parameter.
Instead of calling .containsKey followed by get and put, I prefer to get, check null and then put, whenever possible. This is not always possible, for example when null is a meaningful entry, but that's not the case here.


Based on the above suggestions, the implementation could become a bit simpler:

public Map&lt;String, Integer&gt; findOccurences(String str, int length) {
    Map&lt;String, Integer&gt; map = new HashMap&lt;&gt;();
    int limit = str.length() - length + 1;
    for (int i = 0; i &lt; limit; i++) {
        String sub = str.substring(i, i + length);
        Integer counter = map.get(sub);
        if (counter == null) {
            counter = 0;
        }
        map.put(sub, ++counter);
    }
    return map;
}


And it would be good to add a unit test for it:

@Test
public void test_This_This_() {
    Map&lt;String, Integer&gt; map = new HashMap&lt;&gt;();
    map.put("his", 2);
    map.put("Thi", 2);
    map.put(" Th", 1);
    map.put("s T", 1);
    map.put("is ", 2);
    assertEquals(map, findOccurences("This This ", 3));
}