Find the first reoccuring char from a String

Question

Can anyone give me advice on how I can improve my answer? I have tested it out using "ABCA", "BCABA" and "ABC". Two things I would like help on. 1.How can I move this code into a method where it returns the reoccurring char? I know how to return the char once it has been found but what about the case where there is not char what do I return back in that case? Since I cannot return char. 2. Is using the break statement bad practice? I noticed that the code would with "BCABA" both "B" and "A" are printed.

Thanks.

 public static void main(String[] args) {
 String testString = "BCABA";
 Map<Character, Integer> recurringCharsMap = new HashMap<>();
 for (char character : testString.toCharArray()) {
 if (recurringCharsMap.containsKey(character)) {
 System.out.println("Recurring char is: " + character);
 break;
 } else {
 recurringCharsMap.put(character, 1);
 }
 }
}

Answer 1

• Having a method with Character return type (instead of the primitive char) allows you to return null when no recurring character is found.

• Instead of using break, you can return the first recurring character you find. If none are found, return null.

• You don't need a HashMap (unless you want to count the number of occurrences of each character, which you don't). Use a HashSet instead.

  public static Character findFirstRecurringChar (String input) { 
   Set<Character> recurringChars = new HashSet<>();
   for (char character : input.toCharArray()) {
   if (!recurringChars.add(character)) { // add will return false if character is
   // already in the Set
   return character; // return the first recurring character
   }
   }
   return null; // no recurring characters
  }
  

Then you can call the method with:

public static void main (java.lang.String[] args) {
 System.out.println (findFirstRecurringChar ("BCABA"));
}

• Eran: @Snowbody That's not required in Java 7 and later (Java can infer the parameter type).
• Snowbody: Yep, just saw that in the docs. Time for me to get back up to speed on Java.
• Rocket Pingu: I think it would be better if ‘!recurringChars.add(character)’ is extracted and placed in a method isRecurring(character, recurringChars) (maybe there is a better name for this) to imrpove the readability of the code. Also, instead of returning null, I think you should return a blank character in case there are no recurring characters.

Answer 2

The Character type already has everything you need. It is either a character or the special value null.

For this task you don't need a map, a simple set is enough.

When you move this code into a separate method, using break becomes bad style because you should directly return. In the currect code it's ok.

public static Character findFirstRecurring(String str, char ch) {
 // Left as an exercise
 ...
 return null;
}

Answer 3

Strings in Java are Unicode, stored as UTF-16. This means that Unicode code points from the BMP (U+0000 to U+FFFF) are represented as one char, but Unicode code points from the SMP (U+10000 to U+10FFFF) are represented as a pair of chars from special ranges. For instance, Chinese characters. So if you just look at individual char you may get bad results if there is more than one SMP code point in the string (and if an SMP code point is the one repeated, you'll return a completely wrong value). The proper way to do this is to look at code points.

I'm basing off of Eran's answer.

public static int findFirstRecurringCodePoint (String input) { 
 Set<Integer> usedCodePoints = new HashSet<>(); // int representing a Unicode code point
 for (int i=0; i<input.length(); i++) {
 int codePoint = input.codePointAt(i);
 if (!usedCodePoints.add(codePoint) { // add() will return false if code point is
 // already in the Set
 return codePoint; // return the first recurring character
 }
 if (codePoint > 0xFFFF) {
 i+=1;
 }
 }
 return -1; // no recurring code points
}

• Eran: You should change Set<int> to Set<Integer>. Java doesn't support primitive generic type parameters.