Checking a string for a palindrome (java) (rookie status)

Question

I am trying to check to see if, entered a string, that the string is a palindrome

if it is display something positive if not... something negative (invalid)

I am currently getting the answer invalid both times (no matter what is entered) i'm not quite sure if there is a problem with the for loops or the boolean statement.

    //ACTION FROM BUTTON PERFORMED HERE
private void ButtonActionPerformed(ActionEvent evt) {
    //creating variables
    String myString = textField1.getText();
    int stringLength = myString.length();
    char arrayOne[] = new char[stringLength];
    char arrayTwo[] = new char[stringLength];
    boolean palindrome = false;

    //for loop to setup ARRAY ONE
    for(int i = 0; i < stringLength-1; i++){
        arrayOne[i] = myString.charAt(i);
    }

    //for loop to setup ARRAY TWO
    for(int i = stringLength-1; stringLength-1 > i; i--){
        arrayTwo[i] = myString.charAt(i);
    }

    //for loop checking if array indexes are equivalent in value (char)
    for(int i = 0; i < stringLength-1; i++){
        if(arrayOne[i] != arrayTwo[i]){
            palindrome = false;
        }
        else{
            palindrome = true;
        }
    }

    //assigning text to the text boxes based on boolean palindrome
    if(palindrome == true){
        textField2.setText("Valid");
    }
    if(palindrome ==false){
        textField2.setText("Invalid");
    }
}

}

i think i commented it descently

http://stackoverflow.com/questions/4138827/check-string-for-palindrome?rq=1 — Brian Roach, Feb 22 '13 at 06:29

Aditi · Answer 1 · 2013-02-22T06:39:55.297

3

Change

for(int i = stringLength-1; stringLength-1 > i; i--)

to

for(int i = 0; i < stringLength-1; i++)

and change

for(int i = stringLength-1; i-1 > 0; i--)

to

for(int i = stringLength-1; i-1 >= 0; i--)

EDIT:

That was a debugging fest!!

Here is a working code:

    String myString = textField1.getText();
    int stringLength = myString.length();
    char arrayOne[] = new char[stringLength];
    char arrayTwo[] = new char[stringLength];
    boolean palindrome = true;
    //for loop to setup ARRAY ONE
    for(int i = 0; i <= stringLength-1; i++){
        arrayOne[i] = myString.charAt(i);
    }

    //for loop to setup ARRAY TWO
    for(int i = stringLength-1, pos = 0; i >= 0; i--, pos++){
        arrayTwo[pos] = myString.charAt(i);
    }

    //for loop checking if array indexes are equivalent in value (char)
    for(int i = 0; i <= stringLength-1; i++){
        if(arrayOne[i] != arrayTwo[i]){
            palindrome = false;
            break;
        }
    }

    //assigning text to the text boxes based on boolean palindrome
    if(palindrome == true){
          textField2.setText("Valid");
    }
    else{
        textField2.setText("Invalid");
    }

edited Feb 22 '13 at 06:39

answered Feb 22 '13 at 06:16

Aditi

1,188
2
16
44

If `stringLength` is greater than 1, this will be infinite loop, because `stringLength` is never changed inside the loop. – Mikhail Vladimirov Feb 22 '13 at 06:19
using this gives me an error "string index out of range" if you changed the ">i" to ">0" – relativeLee Feb 22 '13 at 06:20
@LeeHurlbert Right. Not infinite, because it will end up with exception :) – Mikhail Vladimirov Feb 22 '13 at 06:20
@MikhailVladimirov any suggestions? ^-^ :O – relativeLee Feb 22 '13 at 06:21
@LeeHurlbert There were some errors... I have corrected them.. now it's fine. – Aditi Feb 22 '13 at 06:24
@Aditi You should check your code before posting. – Extreme Coders Feb 22 '13 at 06:28
@ExtremeCoders Yes I should. – Aditi Feb 22 '13 at 06:41
@LeeHurlbert Also, there were many more bugs in the code which have been fixed now. See the edit. – Aditi Feb 22 '13 at 06:42
@aditi sir, you're a saint. thank you so much! i know i had a lotta logic bugs and out of limits but i really did (unfortunately) try to understand it now i've got it! – relativeLee Feb 23 '13 at 01:30

Andrew Mao · Answer 2 · 2013-02-22T06:35:32.970

2

I agree with the other answers about your error, but I think a more concise solution would be

boolean isPalindrome(String myString) {    
    int n = myString.length;
    for( int i = 0; i < n/2; i++ )
        if (myString.charAt(i) != myString.charAt(n-i-1)) return false;
    return true;    
}

Your code would now be

private void ButtonActionPerformed(ActionEvent evt) {
    String myString = textField1.getText();     
    textField2.setText( isPalindrome(myString) ? "Valid" : "Invalid" );
}

edited Feb 22 '13 at 06:35

answered Feb 22 '13 at 06:24

Andrew Mao

35,740
23
143
224

i definitely like this - but it doesn't have both of the arrays, should i call this seperately – relativeLee Feb 22 '13 at 06:30
You don't need arrays at all. I suggest not even using them when you can access the characters of the string one by one anyway. – Andrew Mao Feb 22 '13 at 06:33

Mikhail Vladimirov · Answer 3 · 2013-02-22T06:25:24.913

This loop copies all characters except the last one which probably is not what you wanted:

//for loop to setup ARRAY ONE
for(int i = 0; i < stringLength-1; i++){
    arrayOne[i] = myString.charAt(i);
}

It should probably be fixed like this:

//for loop to setup ARRAY ONE
for(int i = 0; i < stringLength; i++)
{
    arrayOne [i] = myString.charAt (i);
}

Body of this loop:

//for loop to setup ARRAY TWO
for (int i = stringLength-1; stringLength-1 > i; i--)
{
    arrayTwo [i] = myString.charAt (i);
}

is never executed, because initial value of i: stringLength - 1 does not satisfy loop condition: stringLength - 1 > i.

You should probably change it to be:

// For loop to setup ARRAY TWO
for (int i = 0; i < stringLength; i++)
{
    arrayTwo [i] = myString.charAt (stringLength - i - 1);
}

Also, after this loop:

// for loop checking if array indexes are equivalent in value (char)
for (int i = 0; i < stringLength-1; i++)
{
    if (arrayOne [i] != arrayTwo [i])
    {
        palindrome = false;
    }
    else
    {
        palindrome = true;
    }
}

variable palindrome will contain result of last comparison only, so if all characters except the last ones were different but last characters were equal, palindrome will be true which is probably not what you wanted. Probably you should change the code like this:

palindrome = true;
for (int i = 0; i < stringLength; i++)
{
    if (arrayOne [i] != arrayTwo [i])
    {
        palindrome = false;
    }
}

Note that I also changed stringLength - 1 to stringLength, otherwise you were ignoring last characters.

I see - then if i were to change the second part "stringLegnth-1>i" to "i>0" would it satisfy the condition or make it forever true — relativeLee, Feb 22 '13 at 06:27

score 1 · Answer 4 · answered Feb 22 '13 at 06:20

1

//for loop to setup ARRAY TWO
for(int i = stringLength-1; stringLength-1 > i; i--){
    arrayTwo[i] = myString.charAt(i);
}

This falls over after the first iteration.

You need to change it to something like:

//for loop to setup ARRAY TWO
for(int i = stringLength-1; i > 0; i--){
    arrayTwo[i] = myString.charAt(i);
}

answered Feb 22 '13 at 06:20

mcalex

6,628
5
50
80

THANK YOU SO MUCH! Worked perfectly and i realized my mistake – relativeLee Feb 22 '13 at 06:24

score 0 · Answer 5 · answered Feb 22 '13 at 06:20

0

The easiest way to test for a palindrome in java

String str = "Able was I ere I saw Elba"
boolean palindrome = str.equalsIgnoreCase(new StringBuilder(str).reverse().toString());

Yep, that's it.

answered Feb 22 '13 at 06:20

Cody A. Ray

5,869
1
37
31

I thank you for this ^_^ however, {and i know weird} i'm actually trying to write the method "check reverse" thanks tho! – relativeLee Feb 22 '13 at 06:24
Should probably also trim() it – gandolf Feb 22 '13 at 06:24
easiest, yes, most efficient, no – bowmore Feb 22 '13 at 06:27

Achintya Jha · Answer 6 · 2013-02-22T06:32:55.253

public static void main(String[] args) {
    String s = "akaq";
    boolean b = false;
    for (int i = 0, j = s.length() - 1; i < j; i++, j--) {
        if (s.charAt(i) == s.charAt(j)) {
            b = true;
            continue;
        } else {
            b = false;
            break;
        }
    }
    if (b)
        System.out.println("Palindrome");
    else
        System.out.println("Not Palindrome");
}

Try something like this instead of 2-3 for loops.

Holden Lewis · Answer 7 · 2013-02-22T06:33:36.703

0

Change the first for loop from stringLength-1 to just stringLength because you are using < and not <=

Change the second for loop to

if(int i = stringLength-1; i>=0; i--)

Also, set palindrome to true by default and remove the

else{
    palindrome = true;
}

part because now if the first and last characters of the loop are the same, but not the middle, it will return true.

EDIT: Also the third for loop should be stringLength and not stringLength-1 because you are using < and not <=

edited Feb 22 '13 at 06:33

answered Feb 22 '13 at 06:26

Holden Lewis

387
3
18

do you mean for(int i = stringLength-1; i>=0; i++) – relativeLee Feb 22 '13 at 06:28
No, because then i would increase forever and never be less than 0. – Holden Lewis Feb 22 '13 at 06:30

score 0 · Answer 8 · answered Feb 22 '13 at 06:34

There's no need to copy everything into arrays. String is basically an array itself. You can access the individual characters with charAt().

Also there is no need to loop the entire length of the String since equality is associative.

So simply use :

public boolean isPalindrome(String s) {
    for (int i = 0; i < s.length() / 2; i++) {                  // only until halfway
        if (s.charAt(i) != s.charAt(s.length() - i - 1)) {      // accessing characters of String directly
            return false;
        }
    }
    return true;
}

One last remark : if the String's length is odd, you don't need to check the middle chracter. So in the code above the diision by

Checking a string for a palindrome (java) (rookie status)

8 Answers8