-1

1.Palindrome code using strings

#include<iostream>
using namespace std;

int main()
{
    int t,n;
    cin>>t;
    while(t--)
    {
        cin>>n;
        string num=to_string(n);
        string rev;
        //Reversed the string using for loop
        for(int i=num.length();i>=0;i--)
        {
            rev+=num[i];
        }
        //Checking the strings if they are same
        cout<<num<<" "<<rev<<endl;

        if(num.compare(rev)==0)
            cout<<"wins";
        else 
            cout<<"loses";
    }
    return 0;
}

2.Output for n=101

101 101 loses

I tried to string method on a separate integer and compare it with other string and it worked. I don't understand why is the compare method not returning 0.

Anees Patel
  • 7
  • 2
  • The problem is not the comparison `num.length()` is the wrong starting position. You are 1 past the end of the string. – drescherjm Jun 21 '22 at 13:25
  • FYI -- You don't need to use strings for this at all, and instead work purely with integer using modulus, division/multiplication and addition. A simple loop that reconstructs the number and then checks for equality is all you need. As a matter of fact, doing things that way would result in a much faster (in terms of runtime) solution. – PaulMcKenzie Jun 21 '22 at 13:38
  • But it's so much simpler to `string rev = num; std::reverse(rev);` Careful with numbers ending in 0 though. Is 1230 and 0321 palindrom? – Goswin von Brederlow Jun 21 '22 at 13:54
  • This doesn't address the question, but `std::cin >> n;` reads text from the console and converts it to an integer value; `std::string num = std::to_string(n);` undoes that conversion. Just skip the middleman: `std::string num; std::cin >> num;`. – Pete Becker Jun 21 '22 at 14:58

1 Answers1

3

You access num out of bounds since i = num.length() points at one character passed the last character in the string. It therefore access the terminating \0 character and that's the first character you'll copy and that's why the strings won't match.

Possible correction:

for(size_t i = num.length(); i--;) {
    rev += num[i];
}

An alternative, less error prone, solution would be to create rev by using reverse iterators from num:

std::string rev(num.rbegin(), num.rend());
Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
  • I used for(int i=num.length-1;i>=0;i++). It worked. Thank you – Anees Patel Jun 21 '22 at 13:29
  • 1
    @AneesPatel You are welcome! Note: `num.length() - 1` works since you use an `int` (instead of the proper `size_t` which is unsigned). If you'd used an unsigned variable, starting with `length()-1` would be dangerous in cases where `length()` may be `0`. – Ted Lyngmo Jun 21 '22 at 13:32
  • @AneesPatel ... and since you're new here, welcome! You may also want to read [What should I do when someone answers my question?](https://stackoverflow.com/help/someone-answers) – Ted Lyngmo Jun 21 '22 at 13:34
  • `some_string[some_string.length()]` is ok (since C++11), its even writeable, you just may not assign anything else than `\0`. I don't think there is UB – 463035818_is_not_an_ai Jun 21 '22 at 14:28
  • @463035818_is_not_a_number I've heard both. UB and not UB and I can understand both. I've also heard that you're not even allowed to write `\0` there. I don't know what optimizations that could break if the compiler is allowed to assume that noone accesses `[num.length()]` or writes to it via `data()` (where accessing it _is_ ok). I can remove the sentense if you are sure enough though :-) – Ted Lyngmo Jun 21 '22 at 14:33
  • 1
    it changed from UB to not UB in C++11 https://en.cppreference.com/w/cpp/string/basic_string/operator_at – 463035818_is_not_an_ai Jun 21 '22 at 14:33
  • @463035818_is_not_a_number Updated (and that even said that assigning `\0` is ok). Nice to get that cleared up. :-) – Ted Lyngmo Jun 21 '22 at 14:35