c++ Palindrome code problems

this code seems to hang up after I type maam in. I have to make a code that i input a string, check to see if it is a palindrome (same backwards as forwards), and output the palindrome. Any help is greatly appreciated.

#include <iostream>
#include <string>
using namespace std;

int main ()
{
    string Palindrome;
    int Dromelength;
    int counter = 1;
    int counterEnd;
    int x = 0;
    string First;
    string Last;
    
    
    
    cout <<"Input Palindrome(statement read the same backwards as forwards):" << endl;
    getline (cin, Palindrome);                              // input is set to string Palindrome
    Dromelength = Palindrome.length();
    counterEnd = Palindrome.length();
    
    
    while (counter <= counterEnd)
    First = Palindrome.substr(x,1);
    Last = Palindrome.substr(Dromelength, Dromelength -1);
    Dromelength = Dromelength - x;
    if (First == Last)
    {
    cout << Last;
    counter++;
    x++;
       
    }
    else
   
    cout << "That is not a Palindrome." << endl;
    
    system("pause");
    return 0;
}

Wow, it would take a while to outline every problem with this code. Your issue is that it will enter an infinite loop, could someone else please go more in depth on all his problems, ynori perhaps? I've got stuff to do.

fixed the infinite loop problem (counterEnd wasnt declared in this code, but was on mine, haha). Appreciate the help folks.

That's way too complicated for a simple palindrome check. I wrote this function in about a minute, and it's nowhere as messy, it doesn't need all those additional variables and it will run faster:

bool is_palindrome(std::string str)
{
	int len = str.length();

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

	return true;
}

I don't even want to give advice how to improve your program because i honestly think its design is too poor to even start working from.

You should really give a program more thought before you start writing it.

stranac wrote:

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

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

	return true;
}

Divide str.length() with two and change the check to i<=len and you will have halved the time :)

Edit: hell, make the parameter a reference instead while at it.

I have to output the Palindrome backwards and it must print backwards because if it isnt a palindrome then the backwards sentence will not match the original sentence. That's why I have the coding the way I do.

And I have all the variables because my professor took half credit off for my last project because I did it all in minimal code and he wants us to make named variables so a dummy could understand the code.

vegeta_man111 wrote: so a dummy could understand the code.

I hope you kindly explained to him this is an extremely stupid and seriously damaging request. Seriously.

I am sadly taking this class online, but at the same time not sad because I am pretty sure I woul dhave been kicked out of class by now because of his methods. I agree with you all on the issues with coding like this, but if I dont follow the monkey's rules, I will be kicked out of college since c++ is my major for my degree.

EDIT: this is the edited code. Infi-loop….

#include <iostream>
#include <string>
using namespace std;

int main ()
{
string Palindrome;
int Dromelength;
int counter = 1;
int x = 0;
string First;
string Last;



cout <<"Input Palindrome(statement read the same backwards as forwards):" << endl;
getline (cin, Palindrome); // input is set to string Palindrome
Dromelength = Palindrome.length() / 2;


while (counter <= Dromelength)
First = Palindrome.substr(x,1);
Last = Palindrome.substr(Palindrome.length() - x, Palindrome.length() - x -1);
if (First == Last)
{
cout << Last;
counter++;
x++;

}
else

cout << "That is not a Palindrome." << endl;

system("pause");
return 0;
}

fixed the problem, but still hitting an infinite loop. So lost. Any tips?

Tips for free:

Programming

1. Use an editor that supports syntax heighlighting

2. Indent your code more consistently Then you would see syntax and (possibly) semanitcal errors easily…

3. Debugging, step through your code and write on a piece of paper what the values are and see Think about your code more thoroughly before compling and running.

Code

a) Just by looking at the expression in the while loop worried me, since if there is no match it continues to execute instead of checking the length of the string entered. So I modified the loop (This isn't very well written but I am tired, so make do):

i) Increment the counter regardless of whether it is a match or not ii) Remove the useless x value and replace with counter in the substring operations iii) Don't subtract the x value from the Dromenumber. Seems a bit of a waste to use strings for single characters, but I cannot think of a simpler solution in C++ WITHOUT copying it in to a character array which is pointless. iv) You also missed some braces around your while loop. v) To avoid this "negitivity" issue, use a work around. vi) You probably didn't want it telling you that it is not a palindrome (string length/2) times, so I took this out of the loop. vii) To further reduce redundant checking, adding an else case to the if inside the while loop to exit the loop. This is in the initial case that if the very first character is NOT equal the very last character of the input string then it will NEVER be a palindrome.

b) As already stated you can half the number of times through that while by dividing the entered string length by 2 since we already know the symmetry - it's adding redundancy since it would otherwise effectively check twice.

c) This is minor but it is often better that you define your variables in a logical order. Just put the variables in groups of types as you define them then it is easier to read.

If there is a further error in the way this works, then it's just because this has reached my threshold of boredom.

Solution WITH annotated corrections

By sticking to the style you chose and not using any further "simplifications" that your professor so hates (as far as I can tell).

#include <iostream>
#include <string>

using namespace std;

int main ()
{

    //c)
    string Palindrome;
    string First;
    string Last;
    int Dromelength;
    int counterEnd;
    int x = 0;
    int counter = 0;

    
    cout << "Input Palindrome (statement read the same backwards as forwards): " << endl;
    getline(cin, Palindrome);
    Dromelength = Palindrome.length();
    counterEnd  = (Dromelength/2);
    
    //b)              \/
    while (counter < counterEnd)
    //a) iv)
    {
    
		//a) ii)
		First 	= Palindrome.substr(counter,1);
		//a) v)                                    \/             \/
		Last 	= Palindrome.substr( (Dromelength - (counter+1)), 1);
		//a) iii) removed unnecessary line
		
		if (First == Last)
		{
		
			cout << Last;
			x++;
		   
		}
		//a) vii)
		else counter = counterEnd;
		
		
		//a) i)
		counter++;
		
	}
	
	//a) vi)
	if(x != counterEnd)	cout << "This is not a palindrome!" << endl;
    
    system("pause");
    return 0;
    
}

Enjoy.

Jim,

EDIT: Fixed! here is the working code

// Chapter X Assignment X
// Corey Hartshorn

#include <iostream>
#include <string>

using namespace std;

int main ()
{

string Palindrome;
string First;
string Last;
int Dromelength;
int counterEnd;
int x = 0;
int counter = 0;


cout << "Input Palindrome (statement read the same backwards as forwards): " << endl;
getline(cin, Palindrome); // input string is value is assigned to Palindrome
Dromelength = Palindrome.length();
counterEnd = Palindrome.length();

while (counter < counterEnd)
      {

      First = Palindrome.substr(counter,1); // gets "first" letter
      Last = Palindrome.substr( (Dromelength - (counter+1)), 1); // gets "last" letter

      if (First == Last) // checks if first and last are the same
         {
         cout << Last; // prints the letter one at a time together
         counter++;
         }
      else 
      {
           cout << Last;  counter++;
      }
     
         
      



      }
      if (First != Last) // checks if it is a palindrome for correct cout
         {
         
         cout << " is not a palindrome." << endl;
         }
      else cout << " is a palindrome." << endl;


system("pause");
return 0;

}

Thanks for all the help folks and especially you j4m32.

This would be tiny and handy…

#include <conio.h>
#include <string.h>
void main ()
{ char str1[]="", str2[]="";
   int loc=0;
   cout << "Enter a string to be checked ";
   cin >> str1;
   for (int i=strlen (str1)-1; i>=0; i--)
         { str2[loc]=str1[i]; loc++;
           }
cout << "The reverse string is " << str2;
if (strcmp (str1,str2)==0)
   {cout << "The given string is A Palindrome ";
    }
else 
  {  cout << "The given string is NOT A Palindrome ";
}
getch();
}```