-2

While trying to check Palindrome String getting this error"String Index out of bound -1"

public static void main(String[]args) {

    String s= "Madam";
    String Temp=s;
    String k=new String();
    //System.out.println(s.length());
    int m=s.length();

    for (int i=5;i>=m;m--) {
        System.out.println(m);
        String t=String.valueOf(s.charAt(m-1) ) ;
        k=k+t;
        System.out.println(k);
    }

    System.out.println(k);

    if (k==Temp) {
        System.out.println("String is Palindrome"+" "+k);   
    } else {
        System.out.println("String is not Palindrome"); 
    }
}
Nikolas Charalambidis
  • 40,893
  • 16
  • 117
  • 183
priyanka srivastava
  • 57
  • 1
  • 2
  • 14

2 Answers2

1

Remove this

for (int i=5;i>=m;m--)

with this

for (int i=s.length();i>0;i--)  
String t=String.valueOf(s.charAt(i-1) ) ;

because length of string is 5 in this case and then index range is 0-4 in this case and you are also accessing 0 index which will give you -1 at this place s.charAt(m-1) so don't traverse 0 index. plus there should be i-- with decrement operator instead of m--

or one line code can also be as

System.out.println(s.equals(new StringBuilder(s).reverse().toString()));
// this will give you boolean result with True or False
// which can be used with conditional statements to make thing concise and clean

but this will not too efficient when string is considerably very large

Pavneet_Singh
  • 36,884
  • 5
  • 53
  • 68
  • Exception "String index out of range: 5 " its giving this exception .This works fine "for (int i = m-1; i >= 0; i--) " – priyanka srivastava Sep 14 '16 at 19:05
  • 1
    The `StringBuilder` approach will definitely be more efficient than calling `String.valueOf`, followed by string concatenation *for each character*. You can make it even more efficient using `s.contentEquals(new StringBuilder(s).reverse())`, unfortunately, it doesn’t work with upper case letter at the beginning like in the OP’s example. By the way, don’t use abbreviations like `S.o.p`. You save a few seconds of typing at the expense of confusing the target audience (especially newbies). – Holger Sep 15 '16 at 16:45
  • @Holger `StringBuilder` is better though i meant that there can be better solutions when string becomes too large but some newbie don;t like algos and about `S.o.p` i actually knew this at my earlier stage so i thought i could have a single line leverage although i shouldn't have.thanks for pointing this out and for your time too. – Pavneet_Singh Sep 15 '16 at 17:24
0

In the loop you want to use i, but dealing with m. Even you are decreasing m. Whenever m is decreased to 0, s.charAt(m-1) is trying to find character in negative position of the string. As a result, you are getting StringIndexOutOfBoundsException. So, instead of

for (int i=5;i>=m;m--) {
    System.out.println(m);
    String t=String.valueOf(s.charAt(m-1) ) ;
    k=k+t;
    System.out.println(k);
}

It should be:

for (int i = m-1; i >= 0; i--) {
    String t = String.valueOf(s.charAt(i)) ;
    k=k+t;
    System.out.println(k);
}

Simply the code could be:

String s = "Madam", reverse = "";
int m = s.length();

for (int i = m - 1; i >= 0; i--) {
    reverse += s.charAt(i);
}
System.out.println(reverse);

if (reverse.equalsIgnoreCase(s)) { // don't use '==' for checking equality of strings
    System.out.println(s + " is Palindrome");
} else {
    System.out.println(s + " is not Palindrome");
}
Shahid
  • 2,288
  • 1
  • 14
  • 24