0

I am trying to make a test of whether an inputted word is a palindrome or not (the same spelled forward and backward). From what I can see it should work but Eclipse says "The value of the local variable isPalindrome is not used" , but it is used. The problem is that even if the word is not a palindrome it says it is.

import java.util.Scanner;

public class Palindrome {
    public static void main(String[] args) {
        String phrase;
        char[] phraseLetters;
        int endChar;
        boolean isPalindrome;

        Scanner input = new Scanner(System.in);
        System.out.println("Enter a word or phrase.");
        phrase = input.nextLine();
        input.close();

        phrase = phrase.toLowerCase();
        phrase = phrase.replaceAll(" ","");
        phraseLetters = phrase.toCharArray();

        endChar = phraseLetters.length - 1;

        for (int i = 0; i < phraseLetters.length; i++) {
            if (phraseLetters[i] != phraseLetters[endChar]) {
                isPalindrome = false;   
            } else {
                isPalindrome = true;
                endChar -= 1;
            }
        }

        if (isPalindrome = true) {
            System.out.println("This word or phrase entered is a palindrome.");
        } else {
            System.out.println("This word or phrase is not a palindrome.");
        }
    }
}

EDIT: I have tried the if statement being

    if (isPalindrome == true) 

and

    if (isPalindrome)

In both cases Eclipse says "The local variable isPalindrome may not have been initialized," in this if condition.

FINAL EDIT:

I have since moved on, and rewritten this code, however I just went back and fixed my original code if anyone still looks at this.

I initialized isPalindrome at the beginning of the code:

Boolean isPalinddrome = True;

I changed the for-loop condition to:

for (int i = 0; (i < phraseLetters.length) && (isPalindrome); i++)

Finally I changed if (isPalindrome = true) to if (isPalindrome)

Ferret9
  • 101
  • 1
  • 12

4 Answers4

5

if (isPalindrome = true) should be if (isPalindrome == true) (or if (isPalindrome) better! Actually this error is another good reason why not asking if someBoolean == true which is a bad style)

By typing if (isPalindrome = true) you're assigning, again, the value true to the variable isPalindrome. And since you're only assigning value to it, the compiler warns you about unused variable.

It's also good to know this:

At run time, the result of the assignment expression is the value of the variable after the assignment has occurred. The result of an assignment expression is not itself a variable.

So, when you do if (isPalindrome = true) then the if condition is always satisfied.

Maroun
  • 94,125
  • 30
  • 188
  • 241
  • 1
    actually it should be `if(isPalindrome)` `isPalindrome` already has a boolean value, the extra comparison is unneeded. – Hunter McMillen Mar 24 '13 at 22:56
  • 1
    Indeed. I just wanted him to know that when comparing, he should use `==`. (I edited the answer for booleans) – Maroun Mar 24 '13 at 23:00
2

You should assing some boolean value to isPalindrome in the main scope.

For example:

boolean isPalindrome = true
Osvaldo Costa
  • 128
  • 1
  • 1
  • 5
  • 1
    Actually, it does make some sense... If the OP fixes the case with the incorrect assignment at the end (where he assigns true instead of testing for it), he will run into a new error: "Palindrome.java:30: variable isPalindrome might not have been initialized". The reason is that if he doesn't enter the for loop, it is not initialized. – Fredrik Mar 25 '13 at 06:42
1

You have a typo.

if (isPalindrome = true)
{
    System.out.println("This word or phrase entered is a palindrome.");
}
else
{
    System.out.println("This word or phrase is not a palindrome.");
}

Look at the if condition. You used = instead of ==. So, you are setting isPalindrome to true, only the true block is executed, and the compiler sees that isPalindrome never matters.

Now, your class has some logic flaws and some programming traps.

  1. If the first and last characters are not equal, isPalindrome is set to false, and then the program continues. Break out of the loop; don't let isPalindrome be set to true later. Incidentally, your version actually cares only about the first and last characters.
  2. Don't write if (x == true). Just write if (x).
  3. Don't name your boolean isAnything. After all, you may do this in a JavaBean class, and then you'll end up with a method named isIsAnything or getIsAnything. This will annoy your readers.
  4. In the future, don't write all your code in the main(String[]) method. Have the main method use the arguments to construct an instance of the class, and use that instance. This way, you can write unit tests for the class; you can't for main. You can break the code into a few methods. One checks for being a palindrome, while another provides the human-readable output.
  5. It's actually a bad idea to use the no-argument forms of String.toLowerCase() and String.toUpperCase() One day, you might need to write an internationalized application, and you will have to deal with the Turkish locale. You might end up mumbling to yourself, “What the heck is a dotless i?”
  6. Don't use i and j as variable names here. Use names that show the variable's purpose. Something like:

    for (int start = 0, end = phraseLetters.length - 1; start < end; start++, end--) { ... }

After all, when start passes end, you're just repeating yourself.

Eric Jablow
  • 7,874
  • 2
  • 22
  • 29
  • Thank you for the tips. I now realize the mistake with the = sign, but now Eclipse says "The local variable isPalindrome may not have been initialized" in the final if condition. – Ferret9 Mar 25 '13 at 00:06
  • That's because the for loop isn't guaranteed to do anything. If the string is empty, the loop will be skipped, so isPalindrome is never set to anything. Initialize it as true; after all, an empty string is a trivial palindrome. – Eric Jablow Mar 25 '13 at 00:26
0

There are an error and bad practice here.

The bad practice is not to initialize the variabale:

boolean isPalindrome = true;

alththough all primitives in java have a default value (for boolean it's false) it's still always better to intialize the varibale explicitly in order enhance code redability.

The error is in the if clause:

if (isPalindrome = true) {

in this line you assign the value and not checking the variabale , all assigment return the value of the assignment meaning that this expression will always return true. Because of this your code always retrun true.

Roman M
  • 8,849
  • 2
  • 15
  • 17