Instead of:
if (!string.IsNullOrEmpty(state) && state.Length == 2)
{
...
}
do this:
bool stateIsValid = !string.IsNullOrEmpty(state) && state.Length == 2;
if (stateIsValid)
how about performance?
Knaģis 11/15/2007 11:43:00 PM
In most of the cases performance penalty will be so small that it can be ignored. On my computer executing this code Int32.MaxValue times gives me ~17 and ~22 seconds respectively.
kostya.ly 11/16/2007 12:20:04 AM
this is also a case to consider if this should be a separate method. in this example, as yourself, will i need to check the validity of the value of state in more than one place? if so, create a method to encapsulate it. then if the logic for validating the state input ever changes, it's in one and only one place.
Stevi 11/16/2007 1:22:06 AM
I like readable code! And I think you should name/design parameters and methods so that they do not need any more explanation. I try to keep my methods short and often use refactoring tools to extract methods. But I don't think you should introduce variables instead of commenting your code. Comments are exellent for improving readability of your code in this kind of situations. No unnecessary code, no performance lost, just simple and readable code. DO! // If state is valid if (!string.IsNullOrEmpty(state) && state.Length == 2) { ... } But don't comment the obvious! DON'T // Declare variable stateIsValid // Set stateIsValid to true if state is valid, otherwise // set stateIsValid to false bool stateIsValid = !string.IsNullOrEmpty(state) && state.Length == 2; // Check if variable stateIsValid equals to true if (stateIsValid) { // Do work if stateIsValid is true ... }
Kristofer Liljeblad 11/16/2007 2:04:48 AM
Kristofer Liljeblad 11/16/2007 2:15:36 AM
Sorry! Just got back to my computer and pressed F5 to refresh the screen. Internet Explorer asked me if I would like to post the data once again and so I did... and now you got 2 identical comments from me. That is something you should tip us how we can avoid in our own sites.
Kristofer Liljeblad 11/16/2007 2:20:14 AM
I think the first example is more readable than the first. I guess I just don't really see how storing the results in a variable makes it more readable.
Luke Foust 11/16/2007 2:31:06 AM
Kristofer, this "F5" problem annoys me as well :) I will definitelly tip on that later. Regarding comments. Although they perfectly do their job, I think it's better to have self-explanatory code which doesn't require comments (if possible of course). Luke, the point is not in storing results in variable. It's about how fast you can understand what this "if" checks. I think it's much easier to understand logic of the following statement: if (stateIsValid) { ... and then if you need to understand what does "valid state" mean, you can review another line: bool stateIsValid = !string.IsNullOrEmpty(state) && state.Length == 2;
kostya.ly 11/16/2007 1:03:12 PM
Just a notice... there is a bug with this method in VS2005 : http://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?FeedbackID=113102
Depechie 11/16/2007 1:36:52 PM
I liked what I saw here until I realized that a better thing would be to do a pull out Method refactoring. In other words, this would be wore easily read as a function than a variable. Why? Because the logical expression is still in the function when using a variable. This causes people like Luke, above, to still spend time running through the conditional logic instead of just reading the intent. I say move the variable out to a method and get it out of the readers view.
David Starr 11/26/2007 10:34:08 PM
Just use common sense when choosing between "extract method" and local variable. What you don't want is to end up with messy class with dozens of methods for each "if" statement.
kostya.ly 11/27/2007 1:16:16 PM
Its worth compromising performance for readability only if the logical condition is either too complex or is used in multiple places, else the code with appropriate comments is just fine without declaring a local variable.
Samuel 12/4/2007 1:04:40 AM
It's worth doing this if you're planning on re-using the logic of the given loop. If you do need to re-use the condition, store it in a local variable, so that it's garbage collected when the method ends and to implement code redundancy. You can also easily check whether the condition is true or not using conditional logic.
Mark Gibbins 1/8/2008 7:57:03 AM