Palindrone tester

[ResidentBiscuit]

How is my palindrone tester deal? I know it's not perfect, I wrote in about 5 minutes.

bool is_palindrone(unsigned long int n)
{
    //Int to string
    std::string strProd = "";
    std::stringstream ss;
    ss << n;
    strProd = ss.str();

    //Creates reversed copy.
    std::string reversed;
    int sz = strProd.size();
    for(int i = sz; i > -1; i--)
    {
        reversed[sz - i] = strProd[i];
    }

    //Checks for palindrone
    if(strProd == reversed)
    {
        return true;
    }
    else
    {
        return false;
    }
}

EDIT:
Doesn't work. I should test things first.

[MrHutch]

Haha. I like the edit. :-)

Also, I think the term is palindrome.

Could you maybe loop backwards through each character of the string and assign that to the new string then compare?

[ResidentBiscuit]

Ah that would explain those damn red lines. Thanks for the catch though!
Anyway, got the reversing to work, but the comparison isn't working. Here's the updated function.

bool is_palindrone(unsigned long int n)
{
    //Int to string
    std::string strProd = "";
    std::stringstream ss;
    ss << n;
    strProd = ss.str();

    //Creates reversed copy.
    std::string reversed;
    int sz = strProd.size();
    for(int i = sz; i > -1; i--)
    {
        reversed.push_back(strProd[i]);
        std::cout << reversed << "\n";
    }

    //Checks for palindrone
    if(strProd.compare(reversed) == 0)
    {
        return true;
    }
    else
    {
        return false;
    }
}

Running it prints out each step of the reverse, and the end product is the reversed copy of the original, so some reason the check at the end is failing.

EDIT:
When trying to actually output it like I have now, in the original way, it caused a crash. Probably because on iteration one, the string is uninitialized. I'm assuming, if someone wants to shed some light there that would be cool

[ResidentBiscuit]

Next attempt. Not sure why it keeps returning false, probably some small thing I'm missing like always.

This is replacing the current check for palindrome section:

//Checks for palindrome
    bool palindrome = true;
    for(unsigned int i = 0; i < strProd.size(); i++)
    {
        if(strProd[i] != reversed[i])
        {
            palindrome = false;
            break;
        }
    }
    return palindrome;

[MrHutch]

I think i might need to be one less in the first iteration maybe?

Here's my crack anyway (ignore the dodgy indentation - VS2010):

bool IsPalindrome(unsigned long int n)
{
   //Int to string
   string my_str,
          rev_str;

   stringstream ss;

   ss << n;
   my_str = ss.str();

   int lc = my_str.length();
   while(lc--)
   {
      rev_str.append(my_str.substr(lc, 1));
   }

   cout << "my_str: " << my_str << "\trev: " << rev_str << endl;
   
   if(my_str == rev_str)
      return true;
   else
      return false;
}

Edit: Fixed indentation.

[ResidentBiscuit]

Hmm, we seem to accomplish the same thing in different ways. I dont know if you saw, but I added some output to check to make sure it's getting reversed, and it is.

Does your check work?

[MrHutch]

Yeah, seems to. Returns 1 when I enter a palindrome, 0 if not.

Not sure why your check isn't working if both string are the same when printed. No odd spaces or anything?

Edit: Getting some problems with bigger numbers, I think. I shall investigate further.

[ResidentBiscuit]

Found the error. I thought maybe it was adding something hidden in there (ie a space or something) and found it. Here's an output:

Enter a number: 76767
        1
 7      2
 76     3
 767    4
 7676   5
 76767  6

Anyway here's the line of code I added.

std::cout << reversed << "\t" << reversed.length() << "\n";

In the loop that reverses the string. So I think you were right.

[helios]

Now do it without reversing the string.

[ResidentBiscuit]

@helios,

I had that thought when I started it, but had never done and only had a little bit of time left in the day. After I get this done, im gonna go back and do it that way. I feel like its gonna be ugly though

[MrHutch]

How about this? I haven't fully tested it yet, though. Ignore stupid indentation.

bool NoReverse(string str)
{
	int len = str.length();

	if(len%2 == 0)
	{
		// Even length
		cout << "Even length\n";
		for(int i=0; i < (len/2)+1; i++)
		{	
			if(str.substr(i,1) != str.substr(len-1-i,1))
				return false;
		}
	}
	else
	{
		// Odd length
		cout << "Odd length\n";
		for(int i=0; i != (len/2)+1; i++)
		{
			if(str.substr(i,1) != str.substr(len-1-i,1))
				return false;
		}
	}
	
	return true;
}

Edit: Passed a string to omit the stringstream stuff. Obviously now it'll work on any characters, not just digits. For only digits, some sort of isdigit check could be included before checking the string.

[helios]

iHutch105: That's terrible.
* What do you use the parity check for?
* Don't use std::string::substr().

[ResidentBiscuit]

Figured it out. I'm pretty sure I was grabbing the \0 character at the end of the primary string, and sticking it in the first spot of the reversed string.

//Creates reversed copy.
    std::string reversed;
    int sz = strProd.size();
    for(int i = sz-1; i > -1; i--)
    {
        reversed.push_back(strProd[i]);
        std::cout << reversed << "\n";
    }

Changed it to start at sz-1

ASFAIK, this works. It's worked for the few test cases I've given it. Let me know what you think/where it can be improved.

[MrHutch]

Whoops, didn't realise how similar my loops got there as I worked on them separately.

Why not use substr, though?

This better?

bool NoReverse(string str)
{
	int len = str.length();

	for(int i=0; i < (len/2)+1; i++)
	{	
		if(str[i] != str[len-1-i])
			return false;
	}
	
	return true;
}

[ResidentBiscuit]

I am curious what's wrong with substr() here. I personally think it's less readable, but is there an actual issue with it?

[MrHutch]

I guess it is a little less readable, yeah.

It's a work habit I've picked up - we have our own string class with a substr function that does some sort of error checking.

[Peter87]

I think helios don't like to use substr here because it's unnecessary slow.

[helios]

substr() allocates memory. Using it to make string comparisons just reeks of laziness.

[MrHutch]

Oh, ok, I see.

Is the later post preferred?

[ResidentBiscuit]

Cool, finished Euler #4. Was pretty easy. Though, what I have right now seems pretty inefficient. It only takes me 2.8 seconds, but I know there is a lot of unneccesary calculations in there. Here's what I got:

unsigned long int product = 0, answer = 0;
    int operand1 = 0, operand2 = 0;
    for(unsigned int i = 999; i > 0; i--)
    {
        for(unsigned int j = i; j > 0; j--)
        {
            product = i * j;
            if(is_palindrome(product) && product > answer)
            {
                answer = product;
                operand1 = i;
                operand2 = j;
            }
        }
    }

    std::cout << "Answer is: " << answer << "\tProduct of " << operand1 << " and " << operand2;

Any ideas on how to cut down on the calculations that aren't needed? Originally, I had it stop at the first palindrome it found, but that wasn't right. And I see why now.