Fixing a looping issue for removing letters in a String

Instead of using complex algorithms and loops like this you can just use HashSet which will work just like a list but it won't allow any duplicate elements.

private static String removeDuplicateWords(String str) {

        HashSet<Character> xChars = new LinkedHashSet<>();
        for(char c: str.toCharArray()) {
            xChars.add(c);
        }

        StringBuilder sb = new StringBuilder();
        for (char c: xChars) {
            sb.append(c);
        }

        return sb.toString();
    }

So you actually want to remove all occurrences that appear more than once entirely and not just the duplicate appearances (while preserving one instance)?

"Yea that’s exactly right "

In that case your idea won't cut it because your duplicate letter detection can only detect continuous sequences of duplicates. A very simple way would be to use 2 sets in order to identify unique letters in one pass.

public class RemoveLettersSeenMultipleTimes {

     public static void main(String []args){
        String input = "abcabdgag";
        
        Set<Character> lettersSeenOnce = lettersSeenOnceIn(input);
        
        StringBuilder output = new StringBuilder(); 
        
        for (Character c : lettersSeenOnce) {
          output.append(c);
        }
        
        System.out.println(output);
        
     }
     
     private static Set<Character> lettersSeenOnceIn(String input) {
        Set<Character> seenOnce = new LinkedHashSet<>();
        Set<Character> seenMany = new HashSet<>();
        
        for (Character c : input.toCharArray()) {
            if (seenOnce.contains(c)) {
                seenMany.add(c);
                seenOnce.remove(c);
                continue;
            }
            
            if (!seenMany.contains(c)) {
                seenOnce.add(c);
            }
        }
        
        return seenOnce;
     } 
}

Say you feed the program with the input "AAABBC", then the expected output should be "ABC".

Now in the for-loop, i gets incremented from 0 to 5.

After 1st iteration: kb becomes AABBC and i becomes 5 + 0 = 5 and gets incremented to 6. And now the condition for the for-loop is that i < kb.length() which equates to 6 < 5 returning false. Hence the for-loop ends after just one iteration.

So the problematic line of code is i = kb.length() + i; and also the loop condition keeps changing as the size of kb changes.

I would suggest using a while loop like the following example if you don't worry too much about the efficiency.

public static void main(String[] arg) {
        String kb = "XYYYXAC";
        int i = 0;

        while (i < kb.length()) {
            char temp = kb.charAt(i);
            for (int j = i + 1; j < kb.length(); j++) {
                char dup = kb.charAt(j);
                if (temp == dup) {
                    kb = removeCharByIndex(kb, j);
                    j--;
                }
            }
            i++;
        }
        System.out.println(kb);
}

private static String removeCharByIndex(String str, int index) {
        return new StringBuilder(str).deleteCharAt(index).toString();
}

Output: XYAC

EDIT: I misunderstood your requirements. So looking at the above comments, you want all the duplicates and the target character removed. So the above code can be changed like this.

public static void main(String[] arg) {
        String kb = "XYYYXAC";
        int i = 0;

        while (i < kb.length()) {
            char temp = kb.charAt(i);
            boolean hasDup = false;
            for (int j = i + 1; j < kb.length(); j++) {
                if (temp == kb.charAt(j)) {
                    hasDup = true;
                    kb = removeCharByIndex(kb, j);
                    j--;
                }
            }
            if (hasDup) {
                kb = removeCharByIndex(kb, i);
                i--;
            }
                
            i++;
        }
        System.out.println(kb);
}

private static String removeCharByIndex(String str, int index) {
        return new StringBuilder(str).deleteCharAt(index).toString();
}

Output: AC

Although, this is not the best and definitely not an efficient solution to this, I think you can get the idea of iterating the input string character by character and removing it if it has duplicates.

There are a few problems here:

Problem 1

for (i = 0; i < kb.length(); i++) {

should be

for (i = 0; i < kb.length() - 1; i++) {

Because this

if (temp == kb.charAt(i+1))

will explode with an ArrayIndexOutOfBoundsException otherwise.

Problem 2

Delete this line:

i = kb.length() + i;

I don't understand what the intention is there, but nevertheless it must be deleted.

Problem 3

Rather than lots of code, there's a one-line solution:

String deduped = kb.replaceAll("[" + input.replaceAll("(.)(?=.*\\1)|.", "$1") + "]", "");

This works by:

• finding all dupe chars via input.replaceAll("(.)(?=.*\\1)|.", "$1"), which in turn works by consuming every character, either capturing it as group 1 if it has a dupe or just consuming it if a non-dupe
• building a regex character class from the dupes, which is used to delete them all (replace with a blank)

The following answer concerns only the transformation of XYYYXACX to ACX. If we wanted to have AC, it's a whole different answer. The other answers already speak about it, and I'll invite you to consult the contains method of String too.

We should consider avoiding -most of the time- modifying the things we iterate. Using a temporary variable could be a kind of solution. To use it, we could change our mindset. Instead of erasing the undesired letters, we can save the ones we want.

To identify the desired character, we need to test if all surrounding letters are different from the tested one. It'll be the opposite of what you did with if(temp == kb.charAt(i+1)) { like if(temp != kb.charAt(i+1)) {. But considering that the tested string will not change anymore, we will need to test the previous letter too as if(temp != kb.charAt(i-1) && temp != kb.charAt(i+1)) {.

As previously said, once we have identified the letter, we will keep the value with a temporary variable. That will lead to replace kb = kb.replace(""+temp, ""); by buffer = buffer + temp; if buffer is our temporary variable initialized with an empty string (Aka. String buffer = "";). In the end, we could override our base value with the temporary one.

At this step, we will have:

public static void main(String[] arg) {
    Scanner scanner = new Scanner(System.in);
    String kb = scanner.nextLine();
    String buffer = "";
    int i;
    for (i = 1; i < kb.length(); i++) {
        char temp = kb.charAt(i);
        if(temp != kb.charAt(i-1) && temp != kb.charAt(i+1)) {
            buffer = buffer + temp;
        }
    }
    kb = buffer;
    System.out.println(kb);
}

That'll sadly not work, trying to access invalid indexes of our string. We should consider two particular behavior for the first and the last letter because they are close to only one letter. For these letters, we will have only one comparison. So, we can make them inside or outside the loop. For clarity, we will do it outside. For the first one, it will look like to if (kb.charAt(0) != kb.charAt(1)) { and at if (kb.charAt(kb.length() - 1) != kb.charAt(kb.length() - 2)) { for the last. The body of the condition will remain the same as the one in the loop. Once done, we will reduce the scope of our loop to exclude these character with for (i = 1; i < (kb.length() - 1); i++) {.

Now we will have something working, but only for one iteration:

public static void main(String[] arg) {
    Scanner scanner = new Scanner(System.in);
    String kb = scanner.nextLine();
    String buffer = "";
    int i;
    if (kb.charAt(0) != kb.charAt(1)) {
        buffer = buffer + kb.charAt(0);
    }
    for (i = 1; i < (kb.length() - 1); i++) {
        char temp = kb.charAt(i);
        if(temp != kb.charAt(i-1) && temp != kb.charAt(i+1)) {
            buffer = buffer + temp;
        }
    }
    if (kb.charAt(kb.length() - 1) != kb.charAt(kb.length() - 2)) {
        buffer = buffer + kb.charAt(kb.length() - 1);
    }
    kb = buffer;
    System.out.println(kb);
}

XYYYXACX will become XXACX.

Once said, our index problem can occur again if the string has only one letter. However, all of this would have been useless because obviously, we can't have a duplicate letter in this situation. As a fact, we should wrap the whole thing to ensure that we have at least two letters:

public static void main(String[] arg) {
    Scanner scanner = new Scanner(System.in);
    String kb = scanner.nextLine();
    if (kb.length() >= 2) {
        String buffer = "";
        int i;
        if (kb.charAt(0) != kb.charAt(1)) {
            buffer = buffer + kb.charAt(0);
        }
        for (i = 1; i < (kb.length() - 1); i++) {
            char temp = kb.charAt(i);
            if (temp != kb.charAt(i - 1) && temp != kb.charAt(i + 1)) {
                buffer = buffer + temp;
            }
        }
        if (kb.charAt(kb.length() - 1) != kb.charAt(kb.length() - 2)) {
            buffer = buffer + kb.charAt(kb.length() - 1);
        }
        kb = buffer;
    }
    System.out.println(kb);
}

The last thing to do is perform this treatment until we have no more undesired letters. For this task, the do { ... } while ( ... ) seems perfect. We can use for the condition comparison the size of the string. Because when the size of the previous iteration is equal to the temporary variable, we will know that we have finished. We will need to perform this comparison before affecting the value of our temporary variable to the base one. Otherwise, it'll always be the same.

In the end, the following thing should be a potential solution:

public static void main(String[] arg) {
    Scanner scanner = new Scanner(System.in);
    String kb = scanner.nextLine();
    Boolean modified;
    do {
        modified = false;
        if (kb.length() >= 2) {
            String buffer = "";
            int i;
            if (kb.charAt(0) != kb.charAt(1)) {
                buffer = buffer + kb.charAt(0);
            }
            for (i = 1; i < (kb.length() - 1); i++) {
                char temp = kb.charAt(i);
                if (temp != kb.charAt(i - 1) && temp != kb.charAt(i + 1)) {
                    buffer = buffer + temp;
                }
            }
            if (kb.charAt(kb.length() - 1) != kb.charAt(kb.length() - 2)) {
                buffer = buffer + kb.charAt(kb.length() - 1);
            }
            modified = (kb.length() != buffer.length());
            kb = buffer;
        }
    } while (modified);
    System.out.println(kb);
}

Take note that this code is ugly for the sole purpose of the explanation. We should refactor this code. We can improve it a lot for the sake of brevity and, why not, performance.