Skip to main content
Code Review

Return to Answer

added 215 characters in body
Source Link
Deduplicator
  • 19.6k
  • 1
  • 32
  • 65
  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. Synchronizing C++ iostreams with C stdio, as happens by default, is quite expensive. Call std::ios_base::sync_with_stdio(false); to fix that.

  3. 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.

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

  5. 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.

  6. 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.

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

  8. 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 "*https://codereview.stackexchange.com/questions/166121/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.

  9. 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.

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

  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 "*https://codereview.stackexchange.com/questions/166121/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.

  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. Synchronizing C++ iostreams with C stdio, as happens by default, is quite expensive. Call std::ios_base::sync_with_stdio(false); to fix that.

  3. 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.

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

  5. 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.

  6. 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.

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

  8. 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 "*https://codereview.stackexchange.com/questions/166121/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.

  9. 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.

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

Source Link
Deduplicator
  • 19.6k
  • 1
  • 32
  • 65
  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 "*https://codereview.stackexchange.com/questions/166121/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.

lang-cpp

AltStyle によって変換されたページ (->オリジナル) /