Why is it returning the wrong amount of repetitions in the string?

Question

I want to return the characters that are being repeated as well as the number of times it occurs but my output isn't consistent with what I'm expecting as the output.

It's outputting e 6 times when it should be 4 times as well as outputting j 1 time when it should be 2 times. I'm aware I'm returning it the wrong way as well.

What am I doing wrong and how can I fix it?

public static String solution(String s) {
    int i, j, count = 0;

    for(i = 0; i < s.length(); i++) {
        for(j = i + 1; j < s.length(); j++) {
            if(s.charAt(i) == s.charAt(j)) {
                System.out.print(s.charAt(i) + " ");
                count++;
            } 
        }
    }
    System.out.println();
    System.out.println("no duplicates");
    System.out.println("There are " + count + " repetitions");
    return s;
}

public static void main(String args[]) {
    String s = "eeejiofewnj";
    solution(s);
}

output:

e e e e e e j 
no duplicates
There are 7 repititions

Have you tried a debugger? Execute your algorithm by hand with pen & paper for a small example that fails. — MrSmith42, Nov 02 '18 at 15:39

Link19 · Accepted Answer · 2018-11-02T15:49:06.797

So what you are doing wrong is counting for each letter in the string, how many other letters after this one match it.

So for the first e your loop finds 3 matches, for the second e your loop finds 2 matches etc. and adds these all up.

What you want to do is count how many instances of a char there are in a String and then only display the ones that are higher than 1. The way I'd do it is with a map... like this:

public static String solution(String s) {

    Map<Character, Integer> counts = new HashMap<Character, Integer>();

    // Go through each char and make a map of char to their counts. 
    for (char c : s.toCharArray()) {

        // See if the char is already in the map
        Integer count = counts.get(c);
        // if it wasn't then start counting from 1
        if (count == null) {
            count = 0;
        }
        count++;
        // update the count
        counts.put(c, count);
    }

    // now go through the map and print out any chars if their counts are higher than 1 (meaning there's a duplicate)
    for (Entry<Character, Integer> entry : counts.entrySet()) {
        if (entry.getValue() > 1) {
            System.out.println(MessageFormat.format("there are {0} {1}s",
                    entry.getValue(), entry.getKey()));
        }
    }
    return s;
}

public static void main(String args[]) {
    String s = "eeejiofewnj";
    solution(s);
}

Jakg · Answer 2 · 2018-11-02T16:05:26.833

1

Another alternative with Regular Expressions (discussed in more detail here).

public static void solutioniseThis(final String str)
{
    Matcher repeatedMatcher = Pattern.compile("(\\w)\\1+").matcher(str);

    while (repeatedMatcher.find())
    {
        int count = 0;

        Matcher countMatcher = Pattern.compile(Matcher.quoteReplacement(repeatedMatcher.group(1))).matcher(str);

        while (countMatcher.find())
        {
            count++;
        }

        System.out.println(MessageFormat.format("Repeated Character \"{0}\" - found {2} repetitions, {1} sequentially", repeatedMatcher.group(1),
                repeatedMatcher.group(0).length(), count));
    }
}

public static void main(String args[])
{
    solutioniseThis("eeejiofewnj");
}

Produces an output of:

Repeated Character "e" - found 4 repetitions, 3 sequentially

edited Nov 02 '18 at 16:05

answered Nov 02 '18 at 15:53

Jakg

922
12
39

This doesn't quite do what OP is after since it only counts uninterrupted streams of the same character, you would have seen this if you had used OP's example instead of adding your own test case that suited your implementation. – Link19 Nov 02 '18 at 16:00
@Link19 thanks for letting me know. I've amended mine to handle both scenarios as I could see counting both sequential and total occurrences as being a useful extension. – Jakg Nov 02 '18 at 16:06

score 0 · Answer 3 · answered Nov 02 '18 at 15:28

0

You are counting each matching combination. For e (pseudocode):

CharAt(0) == CharAt(1)
CharAt(0) == CharAt(2)
CharAt(0) == CharAt(7)
CharAt(1) == CharAt(2)
CharAt(1) == CharAt(7)
CharAt(2) == CharAt(7)

For j there is only one:

CharAt(3) == CharAt(10)

answered Nov 02 '18 at 15:28

Donat

4,157
3
11
26

I thought it was going 0 1, 1 2, 2 3, 3 4, etc. – sp92 Nov 02 '18 at 15:35
@sp92 why would it be like that? You start with `i == 0`, then `j` starts at `1` and goes all the way to `s.length() - 1`. In the next iteration you have `i == 1`, then `j` starts at `2` and goes all the way to `s.length() - 1` again and so on. So in the end you count all characters in the higher indices multiple times because you traverse the String in its entirety in every iteration. – QBrute Nov 02 '18 at 16:05

Michael · Answer 4 · 2018-11-02T16:08:01.457

Another solution using recursion.

public Map<Character, Integer> countRecursive(final String s) 
{
    final Map<Character, Integer> counts = new HashMap<Character, Integer>();

    if(!s.isEmpty())
    {
        counts.putAll(countRecursive(s.substring(1)));

        final char c = s.charAt(0);

        if(counts.containsKey(c))
        {
            counts.put(c, counts.get(c) + 1);
        }
        else
        {
            counts.put(c, 1);
        }
    }

    return counts;
}

public static void main(String args[]) 
{
    final String s = "eeejiofewnj";
    final Map<Character, Integer> counts = new CountCharacters().countRecursive(s);

    for(Map.Entry<Character, Integer> count : counts.entrySet())
    {
        if (count.getValue() > 1) 
        {
            System.out.println(MessageFormat.format("There are {0} {1}s",
                    count.getValue(), count.getKey()));
        }
    }
}

Doesn't this just return a map of counts? Doesn't filter by ones higher than 1? — Link19, Nov 02 '18 at 16:04

score 0 · Answer 5 · answered Nov 02 '18 at 16:07

hello this simple code also work:

public static void solution(String s) {

    int[] repetitons = new int[128];

    for (int i=0; i<s.length(); i++){
        repetitons[(int)s.charAt(i)]++;
    }

    int count = 0;

    for (int i=0; i<128; i++){
        if (repetitons[i]>1){
            count+=repetitons[i];
            for (int j=0; j<repetitons[i]; j++){
                System.out.print((char)i+" ");
            }
        }
    }

    System.out.println();
    if (count == 0){
        System.out.println("no duplicates");
    } else {
        System.out.println("There are " + count + " repetitions");
    }
}

public static void main(String args[]) {
    solution("eeejiofewnj");
}

score 0 · Answer 6 · edited Nov 02 '18 at 17:49

0

Another alternative with Java 8 and Apache Utils.

final String s = "eeejiofewnj";
new HashSet<>(s.chars().mapToObj(e->(char)e).collect(Collectors.toList())).stream().map(c -> Pair.of(c, StringUtils.countOccurrencesOf(s, "" + "" + c))).filter(count -> count.getRight() > 0).forEach(count -> System.out.println("There are " + count.getRight() + " repetitions of " + count.getLeft()));

edited Nov 02 '18 at 17:49

Michael

3,411
4
25
56

answered Nov 02 '18 at 16:17

overflow

21
3

What on earth is this! What where does StringUtils come from? and this one line only count the occurrences of one letter? I suspect doing this for each individual letter while be inefficient, and also how will you keep track of which letters you've already done it for if you uses a loop? – Link19 Nov 02 '18 at 16:23

Why is it returning the wrong amount of repetitions in the string?

6 Answers6