On 04/22/2012 01:02 AM, Scott Kostyshak wrote:
*From:* Richard Heck [rgh...@comcast.net]
*Sent:* Saturday, April 21, 2012 4:25 PM
>Whitespace was a bit inconsistent in the patch. Possibly some sort of
setting in your editor.
I use vim and made the patch with git diff. I must have done something
strange. Let me know if this happens again so I can figure out what I
did wrong.
Hard to know, as I don't use vim. But if you look at the patch in
something that shows tabs, etc, you'll see it. I'd guess it was
something to do with auto-indent?
I have a couple of questions if you have time but they are just out of
curiosity. Why is it better to have the following outside of the "else
if" ?
static const QString message =
qt_("Press button to check validity...");
I'm not so worried about the micro-optimization that would happen if
it were moved inside (which would happen if for example a user entered
a space and then deleted it and went to a different tab because we
would never use that string) but I'm guessing there's something that
I'm missing. Isn't it more readable to have the string defined and
memory allocated right before it is used?
I guess this is a matter of preference. But somehow it just seemed
cleaner the way it was before.
Also, you removed the micro-optimization for if a user enters multiple
whitespaces. That is, if the user goes to Local Layout and enters
return three times and a tab, then the buttons and message are
unnecessarily set every time. I agree with you for not putting it in
because 1) it makes the code messier; 2) it seems there's a
non-trivial chance that it could be broken in the future by changing
other parts of the code and 3) the gain does not seem that much at
all: we don't expect the user to enter a lot of whitespace at the
start. Am I correct in this analysis?
When I looked at it, I was worried by (2). It's also true that any kind
of optimization here will probably be invisible. That amount of time it
takes to hit keys dwarfs processing time.
Richard
