OK, I've committed this, and made a changed too to the name is_valid_.
Whitespace was a bit inconsistent in the patch. Possibly some sort of
setting in your editor.
Richard
On 04/21/2012 04:41 AM, Scott Kostyshak wrote:
*From:* Richard Heck [rgh...@comcast.net]
*Sent:* Friday, April 20, 2012 9:54 AM
>This seems sensible, overall. Minor comments below.
>It's a micro-optimization, but it might also make it more
understandable not to do this every time, but to check is_valid_
first: If that's false, we shouldn't need to reset the text, etc.,
because it should >already be correct. So the idea is:
> } else if (!is_valid) {
Thank you for taking a look at this and for your suggestion, Richard.
The attached patch introduces two micro-optimizations. One of them is
what you were getting at: if we are just waiting for the user to
finish entering his input, then nothing should need updating. I proxy
for this state by conditioning on !validatePB->isEnabled(). This is
not perfectly readable but I do think it is intuitive enough. The
condition as a whole could be read as "if the validate button is not
active and the text has changed and the text is not empty then we have
to update."
The second micro-optimization is for if the user had entered
whitespace and kept entering whitespace. This could happen if the user
wanted to hit a few returns and a tab or some spaces before writing
his layout.
There are more micro-optimizations that could be implemented. For
example, the code currently updates everything if it updates anything.
There are cases where only validLB needs updating. I don't know enough
to know if this could make a difference.
One minor point is that the name is_valid_ seems a little misleading.
It does not tell you whether the input is valid. For example,
something that has is_valid_ == FALSE can be a valid entry. The only
name I can think of now is "is_validated". This would get at the idea
that if is_validated == FALSE we don't know if the input is valid or
not because we haven't validated it yet. I'm not too happy with that
either though because if is_validated == TRUE then one might expect
that it went through the function validate(), but that's not
necessarily true.
Comments?
Thanks,
Scott
