Palindrome testCheck if a string is palindrome or two strings are the opposite of each otherMilliseconds to Time string & Time string to MillisecondsFind palindrome in a stringComparing a string using a stackPalindrome evaluator in C++Test if a string is a palindromeCheck for Palindrome string in JavaPalindrome from all the substringsUnit test cases for Python palindromeC++ check if PalindromeMatch a simple balanced language using a queue
Can you perfectly wrap a cube with this blocky shape?
Why is "dark" an adverb in this sentence?
Why does the Earth have a z-component at the start of the J2000 epoch?
Why does FFmpeg choose 10+20+20 ms instead of an even 16 ms for 60 fps GIF images?
Why limit to revolvers?
Is there a good program to play chess online in ubuntu?
Sending a photo of my bank account card to the future employer
What are some symbols representing peasants/oppressed persons fighting back?
Why isn't aluminium involved in biological processes?
A scene of Jimmy diversity
Doing research in academia and not liking competition
I have accepted an internship offer. Should I inform companies I have applied to that have not gotten back to me yet?
Can a Resident Assistant Be Told to Ignore a Lawful Order?
Draw a line nicely around notes
Too many spies!
Teferi's Time Twist on creature with +1/+1 counter
Extension of trace on von Neumann subalgebra
Was Willow's first magic display (blazing arrow through arm) actual magic, and if not, what's the trick?
FPGA CPU's, how to find the max speed?
Why did Steve Rogers choose Sam in Endgame?
Accidentally deleted python and yum is not working in centos7
What is this called? A tube flange bearing threaded for threaded pushrod
pgfkeys: .store in constructed macro
Was all the fuel expended in each stage of a Saturn V launch?
Palindrome test
Check if a string is palindrome or two strings are the opposite of each otherMilliseconds to Time string & Time string to MillisecondsFind palindrome in a stringComparing a string using a stackPalindrome evaluator in C++Test if a string is a palindromeCheck for Palindrome string in JavaPalindrome from all the substringsUnit test cases for Python palindromeC++ check if PalindromeMatch a simple balanced language using a queue
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;
$begingroup$
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;
c++ performance beginner strings palindrome
$endgroup$
add a comment |
$begingroup$
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;
c++ performance beginner strings palindrome
$endgroup$
$begingroup$
Is there a way to combine stack top/pop and queue front/pop in a single statement?
$endgroup$
– dfhwze
Jul 7 at 16:14
6
$begingroup$
As an aside, this is nearly a complete program. Consider giving the full program next time in a similar situation (changing this now is inadvisable).
$endgroup$
– Deduplicator
Jul 7 at 19:08
add a comment |
$begingroup$
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;
c++ performance beginner strings palindrome
$endgroup$
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;
c++ performance beginner strings palindrome
c++ performance beginner strings palindrome
edited Jul 8 at 7:22
Toby Speight
30.8k7 gold badges45 silver badges133 bronze badges
30.8k7 gold badges45 silver badges133 bronze badges
asked Jul 7 at 15:37
Avantika PAvantika P
204 bronze badges
204 bronze badges
$begingroup$
Is there a way to combine stack top/pop and queue front/pop in a single statement?
$endgroup$
– dfhwze
Jul 7 at 16:14
6
$begingroup$
As an aside, this is nearly a complete program. Consider giving the full program next time in a similar situation (changing this now is inadvisable).
$endgroup$
– Deduplicator
Jul 7 at 19:08
add a comment |
$begingroup$
Is there a way to combine stack top/pop and queue front/pop in a single statement?
$endgroup$
– dfhwze
Jul 7 at 16:14
6
$begingroup$
As an aside, this is nearly a complete program. Consider giving the full program next time in a similar situation (changing this now is inadvisable).
$endgroup$
– Deduplicator
Jul 7 at 19:08
$begingroup$
Is there a way to combine stack top/pop and queue front/pop in a single statement?
$endgroup$
– dfhwze
Jul 7 at 16:14
$begingroup$
Is there a way to combine stack top/pop and queue front/pop in a single statement?
$endgroup$
– dfhwze
Jul 7 at 16:14
6
6
$begingroup$
As an aside, this is nearly a complete program. Consider giving the full program next time in a similar situation (changing this now is inadvisable).
$endgroup$
– Deduplicator
Jul 7 at 19:08
$begingroup$
As an aside, this is nearly a complete program. Consider giving the full program next time in a similar situation (changing this now is inadvisable).
$endgroup$
– Deduplicator
Jul 7 at 19:08
add a comment |
3 Answers
3
active
oldest
votes
$begingroup$
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.Synchronizing C++ iostreams with C stdio, as happens by default, is quite expensive. Call
std::ios_base::sync_with_stdio(false);
to fix that.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, usestd::flush
for explicitness.You assume reading from
std::cin
always succeeds. That's generally unsupportable, please handle failure gracefully.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.You are storing the input twice, once in a
std::queue
and once in astd::stack
. Even only storing it in just onestd::deque
(the underlying implementation for both) would be a considerable improvement.Consider encapsulating the test whether the input is a palindrome into its own reusable function, separate from actually getting it.
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.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.return 0;
is implicit formain()
. Make of that what you will.
$endgroup$
$begingroup$
"You should desist from using std::endl, as spurious manual flushing flushes any pretense at performance down the drain." I can't imagine a context in which this would be an issue. If performance is an issue (e.g. printing in a loop), you shouldn't be printing at all. If performance isn't an issue, there's no reason to delay flushing. It could be useful for when debugging a loop, for example. If your program crashes, you know that your log provides an accurate trace of what happened, because all prints were flushed.
$endgroup$
– Alexander
Jul 8 at 17:49
$begingroup$
@Alexander Not printing at all is certainly faster. But not flushing after every line can also help significantly, even be an order of magnitude or more. The error-stream is unbuffered because there complete output until the crash is paramount.
$endgroup$
– Deduplicator
Jul 8 at 20:19
$begingroup$
@Deduplicator I didn't know that about the error stream, good to know. "But not flushing after every line can also help significantly, even be an order of magnitude or more." but still, it doesn't matter. You shouldn't be printing in a tight loop if performance is an issue. Buffering or not, you shouldn't be doing it at all.
$endgroup$
– Alexander
Jul 8 at 20:33
add a comment |
$begingroup$
I see two improvement points in the code.
- It is better to use getLine() and store the input in char* instead of reading each char and appending to a stack
- 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()
$endgroup$
add a comment |
$begingroup$
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.
I think you hit on an excellent idea right there. I'd read a line of input into a string, the compare the first half of the string to the second half in reverse order.
- You can use
std::getline
to read the string. - You can use
your_string.size() / 2
to get half the length. - You can use
your_string.cbegin()
to get an iterator to the beginning of the string. - You can use
your_string.crbegin()
to get a reverse iterator to the string (one that iterates through from the end to the beginning). - You can use
std::mismatch
to compare the two halves of the string. - As Deduplicator pointed out, you probably want a function that does nothing but check whether a string is a palindrome.
If you wanted to minimize changes to your code, you could just read the string into the deque, then to do the comparison, pop one element from the front, and one element from the back, and compare them. The input was palindromic if and only if all the elements match until the deque has fewer than two elements.
$endgroup$
add a comment |
Your Answer
StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f223680%2fpalindrome-test%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
3 Answers
3
active
oldest
votes
3 Answers
3
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
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.Synchronizing C++ iostreams with C stdio, as happens by default, is quite expensive. Call
std::ios_base::sync_with_stdio(false);
to fix that.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, usestd::flush
for explicitness.You assume reading from
std::cin
always succeeds. That's generally unsupportable, please handle failure gracefully.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.You are storing the input twice, once in a
std::queue
and once in astd::stack
. Even only storing it in just onestd::deque
(the underlying implementation for both) would be a considerable improvement.Consider encapsulating the test whether the input is a palindrome into its own reusable function, separate from actually getting it.
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.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.return 0;
is implicit formain()
. Make of that what you will.
$endgroup$
$begingroup$
"You should desist from using std::endl, as spurious manual flushing flushes any pretense at performance down the drain." I can't imagine a context in which this would be an issue. If performance is an issue (e.g. printing in a loop), you shouldn't be printing at all. If performance isn't an issue, there's no reason to delay flushing. It could be useful for when debugging a loop, for example. If your program crashes, you know that your log provides an accurate trace of what happened, because all prints were flushed.
$endgroup$
– Alexander
Jul 8 at 17:49
$begingroup$
@Alexander Not printing at all is certainly faster. But not flushing after every line can also help significantly, even be an order of magnitude or more. The error-stream is unbuffered because there complete output until the crash is paramount.
$endgroup$
– Deduplicator
Jul 8 at 20:19
$begingroup$
@Deduplicator I didn't know that about the error stream, good to know. "But not flushing after every line can also help significantly, even be an order of magnitude or more." but still, it doesn't matter. You shouldn't be printing in a tight loop if performance is an issue. Buffering or not, you shouldn't be doing it at all.
$endgroup$
– Alexander
Jul 8 at 20:33
add a comment |
$begingroup$
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.Synchronizing C++ iostreams with C stdio, as happens by default, is quite expensive. Call
std::ios_base::sync_with_stdio(false);
to fix that.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, usestd::flush
for explicitness.You assume reading from
std::cin
always succeeds. That's generally unsupportable, please handle failure gracefully.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.You are storing the input twice, once in a
std::queue
and once in astd::stack
. Even only storing it in just onestd::deque
(the underlying implementation for both) would be a considerable improvement.Consider encapsulating the test whether the input is a palindrome into its own reusable function, separate from actually getting it.
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.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.return 0;
is implicit formain()
. Make of that what you will.
$endgroup$
$begingroup$
"You should desist from using std::endl, as spurious manual flushing flushes any pretense at performance down the drain." I can't imagine a context in which this would be an issue. If performance is an issue (e.g. printing in a loop), you shouldn't be printing at all. If performance isn't an issue, there's no reason to delay flushing. It could be useful for when debugging a loop, for example. If your program crashes, you know that your log provides an accurate trace of what happened, because all prints were flushed.
$endgroup$
– Alexander
Jul 8 at 17:49
$begingroup$
@Alexander Not printing at all is certainly faster. But not flushing after every line can also help significantly, even be an order of magnitude or more. The error-stream is unbuffered because there complete output until the crash is paramount.
$endgroup$
– Deduplicator
Jul 8 at 20:19
$begingroup$
@Deduplicator I didn't know that about the error stream, good to know. "But not flushing after every line can also help significantly, even be an order of magnitude or more." but still, it doesn't matter. You shouldn't be printing in a tight loop if performance is an issue. Buffering or not, you shouldn't be doing it at all.
$endgroup$
– Alexander
Jul 8 at 20:33
add a comment |
$begingroup$
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.Synchronizing C++ iostreams with C stdio, as happens by default, is quite expensive. Call
std::ios_base::sync_with_stdio(false);
to fix that.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, usestd::flush
for explicitness.You assume reading from
std::cin
always succeeds. That's generally unsupportable, please handle failure gracefully.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.You are storing the input twice, once in a
std::queue
and once in astd::stack
. Even only storing it in just onestd::deque
(the underlying implementation for both) would be a considerable improvement.Consider encapsulating the test whether the input is a palindrome into its own reusable function, separate from actually getting it.
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.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.return 0;
is implicit formain()
. Make of that what you will.
$endgroup$
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.Synchronizing C++ iostreams with C stdio, as happens by default, is quite expensive. Call
std::ios_base::sync_with_stdio(false);
to fix that.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, usestd::flush
for explicitness.You assume reading from
std::cin
always succeeds. That's generally unsupportable, please handle failure gracefully.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.You are storing the input twice, once in a
std::queue
and once in astd::stack
. Even only storing it in just onestd::deque
(the underlying implementation for both) would be a considerable improvement.Consider encapsulating the test whether the input is a palindrome into its own reusable function, separate from actually getting it.
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.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.return 0;
is implicit formain()
. Make of that what you will.
edited Jul 8 at 20:14
answered Jul 7 at 19:05
DeduplicatorDeduplicator
13.2k20 silver badges55 bronze badges
13.2k20 silver badges55 bronze badges
$begingroup$
"You should desist from using std::endl, as spurious manual flushing flushes any pretense at performance down the drain." I can't imagine a context in which this would be an issue. If performance is an issue (e.g. printing in a loop), you shouldn't be printing at all. If performance isn't an issue, there's no reason to delay flushing. It could be useful for when debugging a loop, for example. If your program crashes, you know that your log provides an accurate trace of what happened, because all prints were flushed.
$endgroup$
– Alexander
Jul 8 at 17:49
$begingroup$
@Alexander Not printing at all is certainly faster. But not flushing after every line can also help significantly, even be an order of magnitude or more. The error-stream is unbuffered because there complete output until the crash is paramount.
$endgroup$
– Deduplicator
Jul 8 at 20:19
$begingroup$
@Deduplicator I didn't know that about the error stream, good to know. "But not flushing after every line can also help significantly, even be an order of magnitude or more." but still, it doesn't matter. You shouldn't be printing in a tight loop if performance is an issue. Buffering or not, you shouldn't be doing it at all.
$endgroup$
– Alexander
Jul 8 at 20:33
add a comment |
$begingroup$
"You should desist from using std::endl, as spurious manual flushing flushes any pretense at performance down the drain." I can't imagine a context in which this would be an issue. If performance is an issue (e.g. printing in a loop), you shouldn't be printing at all. If performance isn't an issue, there's no reason to delay flushing. It could be useful for when debugging a loop, for example. If your program crashes, you know that your log provides an accurate trace of what happened, because all prints were flushed.
$endgroup$
– Alexander
Jul 8 at 17:49
$begingroup$
@Alexander Not printing at all is certainly faster. But not flushing after every line can also help significantly, even be an order of magnitude or more. The error-stream is unbuffered because there complete output until the crash is paramount.
$endgroup$
– Deduplicator
Jul 8 at 20:19
$begingroup$
@Deduplicator I didn't know that about the error stream, good to know. "But not flushing after every line can also help significantly, even be an order of magnitude or more." but still, it doesn't matter. You shouldn't be printing in a tight loop if performance is an issue. Buffering or not, you shouldn't be doing it at all.
$endgroup$
– Alexander
Jul 8 at 20:33
$begingroup$
"You should desist from using std::endl, as spurious manual flushing flushes any pretense at performance down the drain." I can't imagine a context in which this would be an issue. If performance is an issue (e.g. printing in a loop), you shouldn't be printing at all. If performance isn't an issue, there's no reason to delay flushing. It could be useful for when debugging a loop, for example. If your program crashes, you know that your log provides an accurate trace of what happened, because all prints were flushed.
$endgroup$
– Alexander
Jul 8 at 17:49
$begingroup$
"You should desist from using std::endl, as spurious manual flushing flushes any pretense at performance down the drain." I can't imagine a context in which this would be an issue. If performance is an issue (e.g. printing in a loop), you shouldn't be printing at all. If performance isn't an issue, there's no reason to delay flushing. It could be useful for when debugging a loop, for example. If your program crashes, you know that your log provides an accurate trace of what happened, because all prints were flushed.
$endgroup$
– Alexander
Jul 8 at 17:49
$begingroup$
@Alexander Not printing at all is certainly faster. But not flushing after every line can also help significantly, even be an order of magnitude or more. The error-stream is unbuffered because there complete output until the crash is paramount.
$endgroup$
– Deduplicator
Jul 8 at 20:19
$begingroup$
@Alexander Not printing at all is certainly faster. But not flushing after every line can also help significantly, even be an order of magnitude or more. The error-stream is unbuffered because there complete output until the crash is paramount.
$endgroup$
– Deduplicator
Jul 8 at 20:19
$begingroup$
@Deduplicator I didn't know that about the error stream, good to know. "But not flushing after every line can also help significantly, even be an order of magnitude or more." but still, it doesn't matter. You shouldn't be printing in a tight loop if performance is an issue. Buffering or not, you shouldn't be doing it at all.
$endgroup$
– Alexander
Jul 8 at 20:33
$begingroup$
@Deduplicator I didn't know that about the error stream, good to know. "But not flushing after every line can also help significantly, even be an order of magnitude or more." but still, it doesn't matter. You shouldn't be printing in a tight loop if performance is an issue. Buffering or not, you shouldn't be doing it at all.
$endgroup$
– Alexander
Jul 8 at 20:33
add a comment |
$begingroup$
I see two improvement points in the code.
- It is better to use getLine() and store the input in char* instead of reading each char and appending to a stack
- 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()
$endgroup$
add a comment |
$begingroup$
I see two improvement points in the code.
- It is better to use getLine() and store the input in char* instead of reading each char and appending to a stack
- 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()
$endgroup$
add a comment |
$begingroup$
I see two improvement points in the code.
- It is better to use getLine() and store the input in char* instead of reading each char and appending to a stack
- 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()
$endgroup$
I see two improvement points in the code.
- It is better to use getLine() and store the input in char* instead of reading each char and appending to a stack
- 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()
answered Jul 7 at 16:15
Ramanathan GanesanRamanathan Ganesan
4744 silver badges5 bronze badges
4744 silver badges5 bronze badges
add a comment |
add a comment |
$begingroup$
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.
I think you hit on an excellent idea right there. I'd read a line of input into a string, the compare the first half of the string to the second half in reverse order.
- You can use
std::getline
to read the string. - You can use
your_string.size() / 2
to get half the length. - You can use
your_string.cbegin()
to get an iterator to the beginning of the string. - You can use
your_string.crbegin()
to get a reverse iterator to the string (one that iterates through from the end to the beginning). - You can use
std::mismatch
to compare the two halves of the string. - As Deduplicator pointed out, you probably want a function that does nothing but check whether a string is a palindrome.
If you wanted to minimize changes to your code, you could just read the string into the deque, then to do the comparison, pop one element from the front, and one element from the back, and compare them. The input was palindromic if and only if all the elements match until the deque has fewer than two elements.
$endgroup$
add a comment |
$begingroup$
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.
I think you hit on an excellent idea right there. I'd read a line of input into a string, the compare the first half of the string to the second half in reverse order.
- You can use
std::getline
to read the string. - You can use
your_string.size() / 2
to get half the length. - You can use
your_string.cbegin()
to get an iterator to the beginning of the string. - You can use
your_string.crbegin()
to get a reverse iterator to the string (one that iterates through from the end to the beginning). - You can use
std::mismatch
to compare the two halves of the string. - As Deduplicator pointed out, you probably want a function that does nothing but check whether a string is a palindrome.
If you wanted to minimize changes to your code, you could just read the string into the deque, then to do the comparison, pop one element from the front, and one element from the back, and compare them. The input was palindromic if and only if all the elements match until the deque has fewer than two elements.
$endgroup$
add a comment |
$begingroup$
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.
I think you hit on an excellent idea right there. I'd read a line of input into a string, the compare the first half of the string to the second half in reverse order.
- You can use
std::getline
to read the string. - You can use
your_string.size() / 2
to get half the length. - You can use
your_string.cbegin()
to get an iterator to the beginning of the string. - You can use
your_string.crbegin()
to get a reverse iterator to the string (one that iterates through from the end to the beginning). - You can use
std::mismatch
to compare the two halves of the string. - As Deduplicator pointed out, you probably want a function that does nothing but check whether a string is a palindrome.
If you wanted to minimize changes to your code, you could just read the string into the deque, then to do the comparison, pop one element from the front, and one element from the back, and compare them. The input was palindromic if and only if all the elements match until the deque has fewer than two elements.
$endgroup$
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.
I think you hit on an excellent idea right there. I'd read a line of input into a string, the compare the first half of the string to the second half in reverse order.
- You can use
std::getline
to read the string. - You can use
your_string.size() / 2
to get half the length. - You can use
your_string.cbegin()
to get an iterator to the beginning of the string. - You can use
your_string.crbegin()
to get a reverse iterator to the string (one that iterates through from the end to the beginning). - You can use
std::mismatch
to compare the two halves of the string. - As Deduplicator pointed out, you probably want a function that does nothing but check whether a string is a palindrome.
If you wanted to minimize changes to your code, you could just read the string into the deque, then to do the comparison, pop one element from the front, and one element from the back, and compare them. The input was palindromic if and only if all the elements match until the deque has fewer than two elements.
answered Jul 8 at 14:56
Jerry CoffinJerry Coffin
29.5k4 gold badges62 silver badges131 bronze badges
29.5k4 gold badges62 silver badges131 bronze badges
add a comment |
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f223680%2fpalindrome-test%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
$begingroup$
Is there a way to combine stack top/pop and queue front/pop in a single statement?
$endgroup$
– dfhwze
Jul 7 at 16:14
6
$begingroup$
As an aside, this is nearly a complete program. Consider giving the full program next time in a similar situation (changing this now is inadvisable).
$endgroup$
– Deduplicator
Jul 7 at 19:08