Expressive or verbose syntax?
-
Marc Clifton wrote:
saying "if (x==true)" really is redundant.
With all respect due, it really does provide for clearer if statements. For example, take the more real-life scenario below:
if(foo.Hierarchies.Contains(bar) && !bar.Children.TrueForAll(myPredicate)) DoSomethingCrazy(x,y,z);
Looking at that, I really have to study the code carefully so as not to miss anything. Is it asking whether foo's hierarchies contains bar and the bar's children don't pass the criteria by myPredicate? Is that a true or false? If I miss one exclamation point, the code will be logically flawed. That's my main reason for switching syntax; the possiblity for developer error is greatly decreased using explicit is true/is false flags. Compare this to the more verbose, yet clearer replacement:
bool fooContainsBar = (someHierarchy.Hierarchies.Contains(bar) == true);
bool barMeetsCriteria = (bar.Children.TrueForAll(myPredicate) == true);
if (fooContainsBar == true && barMeetsCriteria == false)
{
DoSomethingCrazy(x, y, z);
}It is obvious to any developer, even one unfamiliar with the code what's going on. You can literally read what's going on, rather than trying to infer it with the previous example. I used to be of the persuasion that the former syntax was better because it was quick & dirty and required very little effort to write. But as I come back to code I wrote several months ago, I find myself trying to infer the purpose of the code from these if statements. When I write the code in a more verbose, yet clearer style, there is no having to infer the purpose of the code or find out what's going on. You simply read, "if foo contains bar, and bar's children do not meet the criteria, do something crazy". I find this far superior. Yes, comments can help infer purpose, but comments are not always present or up-to-date, and are obviously not compiler-checked for logical correctness.
Marc Clifton wrote:
And it can lead to errors, like "if (x=true)"
Actually, if you do this in C#, you'll get a compiler warning saying "Assignment in conditional expression is always constant; did you mean to use == instead of = ?". In the project I've been working on for the last 3-4 years, I've never once made that mistake because the compiler will catch it and alert me should I accidentally type one less equal sign.
Tech, life, family, faith:
Testing against
trueis really dangerous, particularly if you have to deal with COM VARIANT_BOOL or MFC style BOOL types as well as the built in bool type. This also applies if you are testing the return values of SDK functions which return != 0 for success. If you're in the habit of testing (function() == true) your code will fail as soon as the result is anything other than 1 (true) or 0 (false). If you must explicitly test against boolean types (and quite honestly seeing code in that style gives me a headache and the urge to refactor the offending code), you'd be better advised to test againstfalse, which has the same value (0) in all three types, and generally indicates failure when returned from SDK functions. [edit] One other thing that's just occurred to me about testing booleans is that each occurrance will generate a Lint warning such as 731 ("Boolean argument to equal/not equal"). If (like me) you've ever worked in an team in which we had to justify every lint message in a formal code review you quickly learn to code in a style which generates far fewer warnings.... [/edit] Anna :rose: Currently working mostly on: Visual Lint :cool: Anna's Place | Tears and Laughter "Be yourself - not what others think you should be" - Marcia Graesch "Anna's just a sexy-looking lesbian tart" - A friend, trying to wind me up. It didn't work. -- modified at 3:35 Saturday 31st December, 2005 -
Actually, I find the second example harder to read. I have to go back to the bool initialization to figure out what the state test is to validate it, and I have to remember that == takes precedence over &&. And I'm especially thrown off by the use of the () in the bool initializer, where for my personal tastes, the parens around ((fooContainsBar==true) && (barMeetsCriteria==false)) is much clearer. [edit]Actually, "if (fooContainsBar && barMeetsCriteria)" is clearest of all, to me. :) [/edit] In the first example, the only improvement I would make would be to implement a FalseForAny method to avoid the !x.TrueForAll.
Judah Himango wrote:
Actually, if you do this in C#, you'll get a compiler warning saying
Yes, I know that, but I still see people ignoring warnings, just as they don't have the discipline to keep comments up to date. But, back to the issue of code clarity, what I'm realizing is that this is probably subjective. But subjective in an operational sense--I didn't have any problems at all with the first code example, and find the second one more difficult. It brings to light that people comprehend code with different efficiencies depending on the coding style, and that's a pretty critical thing when it comes to working in a team environment. Marc Pensieve -- modified at 12:40 Friday 30th December, 2005
I think the key here is that when you're looking at months-old code, you're usually trying to understand what the condition is. The condition is self-explanatory because of the variable names outside the if statement. One can, at a glance, infer what the condition is by looking at the if statement, which cannot be said of the first example, at least not for me.
Marc Clifton wrote:
[edit]Actually, "if (fooContainsBar && barMeetsCriteria)" is clearest of all, to me. :) [/edit]
Actually, that's wrong too. What you meant to say was, "if (fooContainsBar && !barMeetsCriteria)" is clearest to you. Which is ironic, because that is one reason why I prefer == true or == false instead of the presence or absence of exclamation points to symbolize true or false.
-
Kings Tools[^] does this automatically for you. :) Anna :rose: Currently working mostly on: Visual Lint :cool: Anna's Place | Tears and Laughter "Be yourself - not what others think you should be" - Marcia Graesch "Anna's just a sexy-looking lesbian tart" - A friend, trying to wind me up. It didn't work.
Cool! Is there one for C#? It looks like this one is just for C++. Kyosa Jamie Nordmeyer - Taekwondo Yi (2nd) Dan Portland, Oregon, USA
-
Cool! Is there one for C#? It looks like this one is just for C++. Kyosa Jamie Nordmeyer - Taekwondo Yi (2nd) Dan Portland, Oregon, USA
I've not tried it in C#, but given that the add-in is written in VB.NET, I wouldn't be at all surprised if it worked already. Anna :rose: Currently working mostly on: Visual Lint :cool: Anna's Place | Tears and Laughter "Be yourself - not what others think you should be" - Marcia Graesch "Anna's just a sexy-looking lesbian tart" - A friend, trying to wind me up. It didn't work.
-
Testing against
trueis really dangerous, particularly if you have to deal with COM VARIANT_BOOL or MFC style BOOL types as well as the built in bool type. This also applies if you are testing the return values of SDK functions which return != 0 for success. If you're in the habit of testing (function() == true) your code will fail as soon as the result is anything other than 1 (true) or 0 (false). If you must explicitly test against boolean types (and quite honestly seeing code in that style gives me a headache and the urge to refactor the offending code), you'd be better advised to test againstfalse, which has the same value (0) in all three types, and generally indicates failure when returned from SDK functions. [edit] One other thing that's just occurred to me about testing booleans is that each occurrance will generate a Lint warning such as 731 ("Boolean argument to equal/not equal"). If (like me) you've ever worked in an team in which we had to justify every lint message in a formal code review you quickly learn to code in a style which generates far fewer warnings.... [/edit] Anna :rose: Currently working mostly on: Visual Lint :cool: Anna's Place | Tears and Laughter "Be yourself - not what others think you should be" - Marcia Graesch "Anna's just a sexy-looking lesbian tart" - A friend, trying to wind me up. It didn't work. -- modified at 3:35 Saturday 31st December, 2005Since I'm using almost exclusively C#, our concept of bool equalling something other than true or false doesn't apply. :)
-
Since I'm using almost exclusively C#, our concept of bool equalling something other than true or false doesn't apply. :)
Fair enough, although I still think its a very bad habit to get into, especially if you do any interop work. Anna :rose: Currently working mostly on: Visual Lint :cool: Anna's Place | Tears and Laughter "Be yourself - not what others think you should be" - Marcia Graesch "Anna's just a sexy-looking lesbian tart" - A friend, trying to wind me up. It didn't work.
-
Testing against
trueis really dangerous, particularly if you have to deal with COM VARIANT_BOOL or MFC style BOOL types as well as the built in bool type. This also applies if you are testing the return values of SDK functions which return != 0 for success. If you're in the habit of testing (function() == true) your code will fail as soon as the result is anything other than 1 (true) or 0 (false). If you must explicitly test against boolean types (and quite honestly seeing code in that style gives me a headache and the urge to refactor the offending code), you'd be better advised to test againstfalse, which has the same value (0) in all three types, and generally indicates failure when returned from SDK functions. [edit] One other thing that's just occurred to me about testing booleans is that each occurrance will generate a Lint warning such as 731 ("Boolean argument to equal/not equal"). If (like me) you've ever worked in an team in which we had to justify every lint message in a formal code review you quickly learn to code in a style which generates far fewer warnings.... [/edit] Anna :rose: Currently working mostly on: Visual Lint :cool: Anna's Place | Tears and Laughter "Be yourself - not what others think you should be" - Marcia Graesch "Anna's just a sexy-looking lesbian tart" - A friend, trying to wind me up. It didn't work. -- modified at 3:35 Saturday 31st December, 2005Anna-Jayne Metcalfe wrote:
Testing against true is really dangerous, particularly if you have to deal with COM VARIANT_BOOL or MFC style BOOL types as well as the built in bool type. This also applies if you are testing the return values of SDK functions which return != 0 for success.
I tend to prefer a little more verbose than a lot less verbose. But overall, I think consistency is the most important thing. This is something a hate about Microsoft - they are not consistent. While it's true that most Win32 APIs return !=0 for "success", most if not all MSI APIs return ERROR_SUCCESS = 0. I think this is incredibly bad practice.
-
Anna-Jayne Metcalfe wrote:
Testing against true is really dangerous, particularly if you have to deal with COM VARIANT_BOOL or MFC style BOOL types as well as the built in bool type. This also applies if you are testing the return values of SDK functions which return != 0 for success.
I tend to prefer a little more verbose than a lot less verbose. But overall, I think consistency is the most important thing. This is something a hate about Microsoft - they are not consistent. While it's true that most Win32 APIs return !=0 for "success", most if not all MSI APIs return ERROR_SUCCESS = 0. I think this is incredibly bad practice.
rwestgraham wrote:
While it's true that most Win32 APIs return !=0 for "success", most if not all MSI APIs return ERROR_SUCCESS = 0. I think this is incredibly bad practice.
Very true. Lack of consistancy is dangerous. The one that really annoys me is
ShellExecute(): Returns a value greater than 32 if successful, or an error value that is less than or equal to 32 otherwise. The following table lists the error values. The return value is cast as an HINSTANCE for backward compatibility with 16-bit Windows applications. It is not a true HINSTANCE, however. The only thing that can be done with the returned HINSTANCE is to cast it to an int and compare it with the value 32 or one of the error codes below. While I can understand their reasoning to an extent (compatibility with 16 bit apps) this quite simply sucks. Anna :rose: Currently working mostly on: Visual Lint :cool: Anna's Place | Tears and Laughter "Be yourself - not what others think you should be" - Marcia Graesch "Anna's just a sexy-looking lesbian tart" - A friend, trying to wind me up. It didn't work. -
Marc Clifton wrote:
saying "if (x==true)" really is redundant.
With all respect due, it really does provide for clearer if statements. For example, take the more real-life scenario below:
if(foo.Hierarchies.Contains(bar) && !bar.Children.TrueForAll(myPredicate)) DoSomethingCrazy(x,y,z);
Looking at that, I really have to study the code carefully so as not to miss anything. Is it asking whether foo's hierarchies contains bar and the bar's children don't pass the criteria by myPredicate? Is that a true or false? If I miss one exclamation point, the code will be logically flawed. That's my main reason for switching syntax; the possiblity for developer error is greatly decreased using explicit is true/is false flags. Compare this to the more verbose, yet clearer replacement:
bool fooContainsBar = (someHierarchy.Hierarchies.Contains(bar) == true);
bool barMeetsCriteria = (bar.Children.TrueForAll(myPredicate) == true);
if (fooContainsBar == true && barMeetsCriteria == false)
{
DoSomethingCrazy(x, y, z);
}It is obvious to any developer, even one unfamiliar with the code what's going on. You can literally read what's going on, rather than trying to infer it with the previous example. I used to be of the persuasion that the former syntax was better because it was quick & dirty and required very little effort to write. But as I come back to code I wrote several months ago, I find myself trying to infer the purpose of the code from these if statements. When I write the code in a more verbose, yet clearer style, there is no having to infer the purpose of the code or find out what's going on. You simply read, "if foo contains bar, and bar's children do not meet the criteria, do something crazy". I find this far superior. Yes, comments can help infer purpose, but comments are not always present or up-to-date, and are obviously not compiler-checked for logical correctness.
Marc Clifton wrote:
And it can lead to errors, like "if (x=true)"
Actually, if you do this in C#, you'll get a compiler warning saying "Assignment in conditional expression is always constant; did you mean to use == instead of = ?". In the project I've been working on for the last 3-4 years, I've never once made that mistake because the compiler will catch it and alert me should I accidentally type one less equal sign.
Tech, life, family, faith:
Judah Himango wrote:
With all respect due, it really does provide for clearer if statements. For example, take the more real-life scenario below: if(foo.Hierarchies.Contains(bar) && !bar.Children.TrueForAll(myPredicate)) DoSomethingCrazy(x,y,z); .... Looking at that, I really have to study the code carefully so as not to miss anything. Is it asking whether foo's hierarchies contains bar and the bar's children don't pass the criteria by myPredicate? Is that a true or false? If I miss one exclamation point, the code will be logically flawed. That's my main reason for switching syntax; the possiblity for developer error is greatly decreased using explicit is true/is false flags. Compare this to the more verbose, yet clearer replacement: bool fooContainsBar = (someHierarchy.Hierarchies.Contains(bar) == true); bool barMeetsCriteria = (bar.Children.TrueForAll(myPredicate) == true); if (fooContainsBar == true && barMeetsCriteria == false) { DoSomethingCrazy(x, y, z); }
Actually, these code segments will not function the same. In the first example, if the foo.Hierarchies.Contains(bar) returns false, the next method bar.Children.TrueForAll(myPredicate) would never be called while in the latter block, it the are both called prior to the test of the first results. My personal preferences are much along the lines as detailed by Mark, but I do find myself using the braces on single line if's and else's much more lately after working at a place that required them. It really depends on the code though and if there will be many of them in a method or just one or two sets. Readability is great as long as it does not clutter the code or force you into many pages of code. I also use the conditional operator "cond-expr ? expr : expr" often which some tend to stray away from using. But for me, being in the C/C++ world for the best part of two decades and now the C# world, it makes the code much cleaner. Quite often in longer conditionals, I will run my of the first block above as:
if(foo.Hierarchies.Contains(bar) && !bar.Children.TrueForAll(myPredicate)) { DoSomethingCrazy(x,y,z); }I would seldom, if every assign local variables to hold the results simply to make a conditional check unless the conditions were quite complex. Rocky <>< Latest Post: SQL2005 Server Managemnet Studio timeouts! Blog: www.Ro -
Actually, I find the second example harder to read. I have to go back to the bool initialization to figure out what the state test is to validate it, and I have to remember that == takes precedence over &&. And I'm especially thrown off by the use of the () in the bool initializer, where for my personal tastes, the parens around ((fooContainsBar==true) && (barMeetsCriteria==false)) is much clearer. [edit]Actually, "if (fooContainsBar && barMeetsCriteria)" is clearest of all, to me. :) [/edit] In the first example, the only improvement I would make would be to implement a FalseForAny method to avoid the !x.TrueForAll.
Judah Himango wrote:
Actually, if you do this in C#, you'll get a compiler warning saying
Yes, I know that, but I still see people ignoring warnings, just as they don't have the discipline to keep comments up to date. But, back to the issue of code clarity, what I'm realizing is that this is probably subjective. But subjective in an operational sense--I didn't have any problems at all with the first code example, and find the second one more difficult. It brings to light that people comprehend code with different efficiencies depending on the coding style, and that's a pretty critical thing when it comes to working in a team environment. Marc Pensieve -- modified at 12:40 Friday 30th December, 2005
I agree with you about the parentheses. I parenthesize almost every expression involving more than one operator.
Software Zen:
delete this; -
rwestgraham wrote:
While it's true that most Win32 APIs return !=0 for "success", most if not all MSI APIs return ERROR_SUCCESS = 0. I think this is incredibly bad practice.
Very true. Lack of consistancy is dangerous. The one that really annoys me is
ShellExecute(): Returns a value greater than 32 if successful, or an error value that is less than or equal to 32 otherwise. The following table lists the error values. The return value is cast as an HINSTANCE for backward compatibility with 16-bit Windows applications. It is not a true HINSTANCE, however. The only thing that can be done with the returned HINSTANCE is to cast it to an int and compare it with the value 32 or one of the error codes below. While I can understand their reasoning to an extent (compatibility with 16 bit apps) this quite simply sucks. Anna :rose: Currently working mostly on: Visual Lint :cool: Anna's Place | Tears and Laughter "Be yourself - not what others think you should be" - Marcia Graesch "Anna's just a sexy-looking lesbian tart" - A friend, trying to wind me up. It didn't work.Yes. One of the more egregious examples of "backward compatible even if it kills us".
Software Zen:
delete this;