none
Best practices for if statements RRS feed

  • Question

  • To some people this is clearer,

     
    If(a == "test")
    {
    if(b == true)
    return
    if(c == true) 
    return
    }
    blah blah blah...

    and to others (me) this next one is clearer

    If(a == "test" && b == true)
    return
    If(a == "test" && c == true)
    return 
    blah blah blah 

    Does anyone know of a best practice for this, or a rule, or know if there is anything in the MSFT coding styles that addresses this? (I am using "true" but the actual code I am wondering about is comparing objects)

    thanks






    • Edited by danielsn Thursday, April 19, 2018 11:51 PM
    Thursday, April 19, 2018 11:47 PM

Answers

  • Personally, I would find your example much clearer written like this.

    if(a == "test" && (b || c))
    {
       return;
    }

    I have no idea if there are any standards documents on the subject. My personal rule is to do what's clearest to me, especially if I'm the one who will have to maintain the code.

    • Proposed as answer by Fei HuModerator Friday, April 20, 2018 6:15 AM
    • Marked as answer by danielsn Friday, April 20, 2018 3:52 PM
    Friday, April 20, 2018 12:00 AM

All replies

  • Personally, I would find your example much clearer written like this.

    if(a == "test" && (b || c))
    {
       return;
    }

    I have no idea if there are any standards documents on the subject. My personal rule is to do what's clearest to me, especially if I'm the one who will have to maintain the code.

    • Proposed as answer by Fei HuModerator Friday, April 20, 2018 6:15 AM
    • Marked as answer by danielsn Friday, April 20, 2018 3:52 PM
    Friday, April 20, 2018 12:00 AM
  • I vote for Ante's solution. It is the most clear, easiest to read and has all of the logic in one IF statement.

    As far as your two proposed solutions, the first is faster, especially if "a" is a function or method call. (It's being called twice in the second example.)

    I created a little C# example to test... Your first solution and Ante's solution run in almost the exact same time, while your second solution is 60-80% slower. As far as code, the MSIL created for C# has 31 bytes for your first solution, 50 for the second, and Ante's has 39, not that it matters too much! (I would have predicted that Ante's would have been the smallest.)


    Mike Smith TechTrainingNotes.blogspot.com
    Books: SharePoint 2007 2010 Customization for the Site Owner, SharePoint 2010 Security for the Site Owner

    Friday, April 20, 2018 1:18 AM
  • This answer does not apply to everything but you are not asking about everything.

    It depends. If this is for your programs and you are the programmer then there is no such thing as a best practice, do it the way you want to.

    If you are asking what the company standard should be then it depends on whether you want standards for writing programs that beginners understand or programs that experienced people are comfortable with. Some companies might say to never use not and use a null statement (just a semicolon) and an "else" instead of a "!". Experienced programmers might find their way elsewhere with standards like that.

    There is no objective answer. This is a subjective question; a matter of opinion. This should not be a question; the type for this thread should be "Discussion".



    Sam Hobbs
    SimpleSamples.Info

    Friday, April 20, 2018 1:37 AM
  • This particular code, will almost never encounter a time when they should return.  Maybe once per ten thousand.  So, I would be curious if you ran it with it returning first, or not.

    The actual code does not use booleans but compares to a variable about 40 characters long, if you include the scope resolution portion.  

    How did you find out how large the MSIL was for the two solutions?

    and thank you for providing a context to know which is "better" than the other.   

    Friday, April 20, 2018 3:55 PM
  • In this case, it wasn't booleans so it wouldn't have looked as neat, or fit on a single line, but still the best answer for how to do it with the clearest possible code.
    Friday, April 20, 2018 3:56 PM
  • Yeah, I thought it was subjective too.   But I did get two "fairly" objective answers.

    My preference is to not put return statements in code, except at the bottom of the method.  This was not really my preference, but what I learned in school.

    If I break that rule, then I like to keep it obvious what logic is involved, and put it all as simple as possible to see.

    Thank you for your answer Sam.

     

    Friday, April 20, 2018 4:00 PM
  • But I did get two "fairly" objective answers.

    The question would be not be allowed in Stackoverflow.

    If the "objective answers" are based on performance, the difference in performance would be a matter of nanoseconds. People time is much more expensive than processor time. It is wasteful to spend time trying to save a total time of a few nanoseconds.



    Sam Hobbs
    SimpleSamples.Info

    Friday, April 20, 2018 4:36 PM
  • True, the best answer was the one that reduced the complexity, and improved readability, which was what I was asking. 

    Though performance is not an issue in this case, as it would likely never be called more than 5000 times in a second, it is still a context to decide with, and that is what I was looking for to.  

    "The question would not be allowed on stack overflow".  

    Yeah, "best practices for readability" is outside the scope of their mission for sure.  However, you are mistaken about it being wasteful to consider readability.  Some large companies may alter a class more than one hundred times.  Writing code in a clear manner is very important in those cases.

    Also, when debugging code, if it is done one way over another it can make a huge difference.  I tend to write in AND statements as I consider them the clearest.  But clearly I was wrong in this case, and I learned something.  

    I believe if I can write exceptionally clear and easy to read code, I can write code that is almost bug free, not just through QA, but through the lifetime.  So, to me, readability maps up there  with SOLID in importance.


     

     

    Friday, April 20, 2018 8:11 PM
  • However, you are mistaken about it being wasteful to consider readability.  Some large companies may alter a class more than one hundred times. Writing code in a clear manner is very important in those cases.

    I think you are assuming you disagree with me and not trying to understand what I am saying. I said:

    People time is much more expensive than processor time.

    Readability is not what I am saying is wasteful, I was saying the opposite. And I still consider all possible responses to be subjective.



    Sam Hobbs
    SimpleSamples.Info

    Friday, April 20, 2018 8:46 PM
  • Yes, I got what you were saying.

    And I was saying that if a class is rewritten 135 times (which some are) that readability can save many hours of time.  

     



     

     
    Friday, April 20, 2018 11:24 PM
  • As a general rule, it's never a good practice to write

        if( a == true )

    rather than

        if( a )

    Now, this advice dates back from a time before "boolean", when one would occasionally find:

        #define TRUE -1
        #define FALSE 0

    And in that case "if( a == TRUE )" is dangerous because it doesn't handle cases where "a" is neither 0 or -1.  However, I think the spirit is still value.  The "if" statement is designed to test a Boolean expression.  "a" is already a Boolean expression.  You don't add to the readability by adding "== true".


    Tim Roberts, Driver MVP Providenza & Boekelheide, Inc.

    Saturday, April 21, 2018 12:11 AM
  • yes, that is true.   

    I put that to take the place of the real expression, which is long and drawn out.  I did a search of our code base, and people on my team only did  "== true" twice, and both times it was in a lengthy statement.  (maybe five comparisons)

    I believe it should be fairly easy to write an algorithm that always reduced a logical expression to the easiest to read, (and nearly shortest) if one had a proper set of rules for it.   

    I am still trying to understand the rules of what make something readable though, so I couldn't write it.   In the case of "If(A == true) I think it has to do with the same thing swear words do.  Just as swear words demonstrate that your ancestors were conquered people (and thus lower class), I think that "if(A == True) shows that you once might have used Visual Basic, which of course, is almost as taboo as swearing.  (Written with my "just kidding, sort of" font, so don't take this too seriously.))  


    Saturday, April 21, 2018 12:51 AM
  • > How did you find out how large the MSIL was for the two solutions?

    Either use ildasm.exe, or in Visual Studio add a breakpoint anywhere, and then when hit, click Debug, Windows, Disassembly. Don't put of a lot of stock into byte counts, as that is only a hint about performance. It's just a curiosity thing!


    Mike Smith TechTrainingNotes.blogspot.com
    Books: SharePoint 2007 2010 Customization for the Site Owner, SharePoint 2010 Security for the Site Owner

    Saturday, April 21, 2018 4:11 AM
  • Yes, I got what you were saying.

    And I was saying that if a class is rewritten 135 times (which some are) that readability can save many hours of time.

    I doubt that you got it because you say that as if I said something inconsistent with that. You are talking about saving people time. I said that processor time is cheap and people time is expensive yet for some reason you seem to be trying to convince me of something different from what you think I said.


    Sam Hobbs
    SimpleSamples.Info



    Saturday, April 21, 2018 4:22 AM