If statement unexpectedly loops infinitely inside for loop

Question

input: "Who lives in a pineapple under the sea?" expected output: "Who sevil in a elppaenip rednu the sea?" (reverse size 5 and above words)

I think the if statement is the problem but I don't know why it's not working. I tried using the code from the if statement block inside the main method and it works fine.

public class Testing {

    public static void main(String[] args) {
        String sentence = "Who lives in a pineapple under the sea?"; 
        System.out.println(spinWords(sentence)); 
    }
    
      public static String spinWords(String sentence) {
        String[] words = sentence.split(" "); 
        int length = words.length; 
        String spinned = ""; 
        for (int i = 0; i < length; i++) {
            String word = words[i];  
            int wlength = word.length(); 
            if (wlength > 4) {  
                String reversed = ""; 
                for (i = wlength - 1; i >= 0; i--) {
                    reversed += "" + word.charAt(i); 
                }
                spinned += reversed + " ";
            } else {
              spinned += word + " "; 
            }
        }
        spinned = spinned.trim(); 
        return spinned; 
      }
}

This is my first stack overflow question btw and I don't really know what I'm doing. I would also love to see better implementations of this code ( for learning purposes ), thanks.

It is *very* unintuitive to have nested loops, both using `i` as their counter. I would say you almost never want to do that. Change the 2nd nested loop to use a different counter. `for (int j = wlength - 1; j >= 0; j--) {` — Michael, Jul 01 '22 at 11:58
@Michael agreed. As explanation: The ´i´ in the first for loop should increase by each iteration. Insted every time the second for loop is finished the `i` value is back to 0! So the first for loop has to start from the beginning. — Japhei, Jul 01 '22 at 12:03
Thanks everyone. I forgot to change word.charAt(i) so I got an exception, but it works fine now with the new counter. — Genczar Lawrenz Tomines, Jul 01 '22 at 12:13

score 0 · Answer 1 · answered Jul 01 '22 at 12:15

Replace the "if" block with below code. You were using the variable "i" which is used by the first "for" loop. This caused the infinite loop.

    if (wlength > 4) {  
        String reversed  = "";
         for(int j = 0; j < wlength; j++) {
             reversed = word.charAt(j) + reversed;
         }
         spinned += reversed + " ";
    } else {
      spinned += word + " "; 
    }

Nexevis · Answer 2 · 2022-07-01T13:25:57.473

The issue in your code is the duplicate use of the local variable i as others have pointed out. Typically you never want a nested loop counter to use the same variable name as the outer loop counter.

There is a reason why for (int i = wlength - 1; i >= 0; i--) is a compilation error (with the int declaration added). Instead simply use another variable name:

for (int j = wlength - 1; j >= 0; j--)

This will fix the error in the code. However, since you asked for other implementations I thought I would show another option using StringBuilder, which I believe is easier to read:

public static void main(String[] args) {
    String sentence = "Who lives in a pineapple under the sea?";
    System.out.println(spinWords(sentence));
}

public static String spinWords(String sentence) {
    String[] words = sentence.split(" ");
    StringBuilder sb = new StringBuilder();

    for (String str : words) {
        if (str.length() >= 5) {
            sb.append(new StringBuilder(str).reverse());
        } else {
            sb.append(str);
        }
        sb.append(" ");
    }

    return sb.toString().trim();
}

StringBuilder is often used when concatenating a String inside of a loop, see this answer.

Additionally, StringBuilder has the reverse() method which you can use to reverse individual words instead of using a nested loop. You then just use toString to convert the StringBuilder object into a String.

Lastly, I used an enhanced for loop of for (String str : words) which allows you to loop directly on the String values instead of needing to use a loop counter.

score 0 · Answer 3 · answered Jul 01 '22 at 13:32

Here's a one-liner:

public static String spinWords(String sentence) {
    return Arrays.stream(sentence.split(" "))
      .map(word -> word.length() < 5 ? word : new StringBuilder(word).reverse().toString())
      .collect(Collectors.joining(" "));
}

See live demo.

If statement unexpectedly loops infinitely inside for loop

3 Answers3