Xjyuk

This is some code that determines if a string of characters is a palindrome or not. My professor says that there is a performance issue with the program, but I can't quite put my finger on it. Can someone find out the 'performance' issue?

Initially, I thought maybe the process is slower as it uses two memory containers, as opposed to simply comparing two halves of a single string.

int main()

 char c;
 bool check = true;
 stack<char> cstack;
 queue<char> cqueue;
 cout << "Enter a string and press return." << endl;
 cin.get(c);
 while (c != 'n') 
 cstack.push(c);
 cqueue.push(c);
 cin.get(c);
 
 while (check && !cqueue.empty()) 
 if (cstack.top() != cqueue.front())
 check = false;
 cstack.pop();
 cqueue.pop();
 
 if (check)
 cout << "Yes it is!" << endl;
 else
 cout << "No it's not." << endl;
 return 0;

1. While it is not quite definitive, it looks like you use using namespace std;.
   
   That namespace is not designed for wholesale inclusion, being vast and subject to change at the whim of the implementation, aside from providing what is standardised.
   
   Read "Why is “using namespace std” considered bad practice?" for more detail.




2. You should desist from using std::endl, as spurious manual flushing flushes any pretense at performance down the drain.
   
   For those rare cases where it is actually necessary for correctness, use std::flush for explicitness.




3. You assume reading from std::cin always succeeds. That's generally unsupportable, please handle failure gracefully.




4. You are reading character-by-character. Each and every read has significant overhead, which you could simply avoid by using std::getline(). Using the proper abstraction is also significantly more readable.




5. You are storing the input twice, once in a std::queue and once in a std::stack. Even only storing it in just one std::deque (the underlying implementation for both) would be a considerable improvement.




6. Consider encapsulating the test whether the input is a palindrome into its own reusable function, separate from actually getting it.




7. Testing whether something is a palindrome seems a favorite passtime of many beginners.
   
   Thus, there are a myriad posts on how to efficiently and elegantly do that in C++, for example "Check if a string is palindrome or two strings are the opposite of each other".
   
   The important points are avoiding expensive copies, and only comparing each element once.




8. If you want one of two values, conditional on some expression, consider the conditional operator expr ? true_expr : false_expr. It is designed for that.




9. return 0; is implicit for main(). Make of that what you will.

I see two improvement points in the code.

1. It is better to use getLine() and store the input in char* instead of reading each char and appending to a stack


2. It is more than enough to iterate till half of the string as the remaining half is checked in the first half iteration cstack.top() != cqueue.front()