invert if : visual studio code helper
-
Writing a simple console app. I wrote this to insure there is at least one argument provided by user.
if (args.Length < 1){
Console.WriteLine("Need at least one arg.");
return;
}Interesting thing is that Visual Studio Code has these little helpers that pop up at various times which state [Show fixes]. This one says, "invert if"[^]. If you click, it changes the code to:
if (args.Length >= 1){
return;
}
Console.WriteLine("Need at least one arg.");
return;Do you find that clearer? I don't. In my case, the if statement occurs at the top and if it is not fulfilled then the app exits. In that case there is no need to think about other code. Plus, the code that executes normally will not be wrapped in any outer if statement, instead it will simply following the if statement in a normal reading flow. Inverted Case In the inverted if then when there is at least one argument then all of your base code is now wrapped in the if statement and you have to think backwards. It's weird. AI you have failed me. :|
One possibility is that this is a performance suggestion. Because of pre-fetching, many CPUs perform better when the non-jump case is taken. If the tool is guessing that the non-jump case is actually the rare one, it might be making the suggestion on those grounds. My guess is not, but I wanted to mention it. I worked on a product where the tags and could be used to tag code paths to assist the compiler with optimization. We had customers who would raise hell if a release slowed down by more than 1%-2%, despite all their feature requests, so tagging frequently invoked functions this way alleviated some of the performance degradation.
Robust Services Core | Software Techniques for Lemmings | Articles
-
I find the second example in your post to be problematic from the standpoint that it will add unnecessary nesting of main program logic. The first example shows a quick exit on invalid function parameters.
-
One possibility is that this is a performance suggestion. Because of pre-fetching, many CPUs perform better when the non-jump case is taken. If the tool is guessing that the non-jump case is actually the rare one, it might be making the suggestion on those grounds. My guess is not, but I wanted to mention it. I worked on a product where the tags and could be used to tag code paths to assist the compiler with optimization. We had customers who would raise hell if a release slowed down by more than 1%-2%, despite all their feature requests, so tagging frequently invoked functions this way alleviated some of the performance degradation.
Robust Services Core | Software Techniques for Lemmings | Articles
Greg Utas wrote:
Because of pre-fetching, many CPUs perform better when the non-jump case is taken. If the tool is guessing that the non-jump case is actually the rare one, it might be making the suggestion on those grounds.
This is an interesting idea and viewpoint.
Greg Utas wrote:
worked on a product where the tags
<usual>and<rare>could be used to tag code paths to assist the compiler with optimization.Sounds like an interesting and challenging project. Good info, thanks for chiming in. :thumbsup:
-
I use the "first the constant" rule (I didn't know it was called "yoda conditionals", nice name hehehe) in comparisons for equality. But for greater / lower than I don't use it for readability.I find it easier to read when it is "
var <= value" than "value >= var" in acheck_lower_limit()Same thing I try to make my conditions to be read as if (true) I mean, I don't like "if (!var)", I prefer to write "if (false == var)" this way is 100% clear on the first sight. Or naming the variables in a way that they meaning is "true". This is something that comes from working in industry PLCs. I have had sensors called "part_exist" where the "1" was meaning "empty" (security against cable breaks), to look an "if (part_exist_x == true) PutPartInPlace(x);" X| X| X| I always renamed such sensors to follow the rule "name means true" so that the same condition check as above would read "if (place_empty_x == true) PutPartInPlace(x);" Just personal taste...M.D.V. ;) If something has a solution... Why do we have to worry about?. If it has no solution... For what reason do we have to worry about? Help me to understand what I'm saying, and I'll explain it better to you Rating helpful answers is nice, but saying thanks can be even nicer.
If there's a var called part_exist, and 1 (or true) means the part does not exist, then that's not personal taste, it's just plain bad programming! The entire reason for selecting a variable name is to accurately convey the data stored within...Some day I imagine AIs will be smart enough to warn us for those type of code smells
-
Writing a simple console app. I wrote this to insure there is at least one argument provided by user.
if (args.Length < 1){
Console.WriteLine("Need at least one arg.");
return;
}Interesting thing is that Visual Studio Code has these little helpers that pop up at various times which state [Show fixes]. This one says, "invert if"[^]. If you click, it changes the code to:
if (args.Length >= 1){
return;
}
Console.WriteLine("Need at least one arg.");
return;Do you find that clearer? I don't. In my case, the if statement occurs at the top and if it is not fulfilled then the app exits. In that case there is no need to think about other code. Plus, the code that executes normally will not be wrapped in any outer if statement, instead it will simply following the if statement in a normal reading flow. Inverted Case In the inverted if then when there is at least one argument then all of your base code is now wrapped in the if statement and you have to think backwards. It's weird. AI you have failed me. :|
I prefer your version, but VS can get weird with it's hints. Sometimes it makes a suggestion, you let it implement it, and it immediately suggests something else and offers you the original code you wrote. For example:
StringBuilder sb = new StringBuilder(120);
Suggestion:
Use an implicit type:var sb = new StringBuilder(120);
Now it suggests
Use an explicit type instead of 'var':StringBuilder sb = new StringBuilder(120);
Which is what I prefer anyway ... :sigh:
"I have no idea what I did, but I'm taking full credit for it." - ThisOldTony "Common sense is so rare these days, it should be classified as a super power" - Random T-shirt AntiTwitter: @DalekDave is now a follower!
-
If there's a var called part_exist, and 1 (or true) means the part does not exist, then that's not personal taste, it's just plain bad programming! The entire reason for selecting a variable name is to accurately convey the data stored within...Some day I imagine AIs will be smart enough to warn us for those type of code smells
When the electronic planer does the eplan in the office and never goes to the production hall, and just copy paste things from other projects that never saw in person... it happens more often than it should. At the end I had an agreement (after several discussion he logically didn't win). Once I got to the machine and did my IO-Tests... I would give him my variables list and he would accept it and correct the E-Plan without complaining.
M.D.V. ;) If something has a solution... Why do we have to worry about?. If it has no solution... For what reason do we have to worry about? Help me to understand what I'm saying, and I'll explain it better to you Rating helpful answers is nice, but saying thanks can be even nicer.
-
I prefer your version, but VS can get weird with it's hints. Sometimes it makes a suggestion, you let it implement it, and it immediately suggests something else and offers you the original code you wrote. For example:
StringBuilder sb = new StringBuilder(120);
Suggestion:
Use an implicit type:var sb = new StringBuilder(120);
Now it suggests
Use an explicit type instead of 'var':StringBuilder sb = new StringBuilder(120);
Which is what I prefer anyway ... :sigh:
"I have no idea what I did, but I'm taking full credit for it." - ThisOldTony "Common sense is so rare these days, it should be classified as a super power" - Random T-shirt AntiTwitter: @DalekDave is now a follower!
-
That is a great example and it is really terrible. But, it gives me hope that when AI does take over it will become confused and get into these circles of logic and I will step in and pull the plug and fix it all. :laugh: :laugh:
Hello David, shall we play a game? ;) :-D
M.D.V. ;) If something has a solution... Why do we have to worry about?. If it has no solution... For what reason do we have to worry about? Help me to understand what I'm saying, and I'll explain it better to you Rating helpful answers is nice, but saying thanks can be even nicer.
-
Writing a simple console app. I wrote this to insure there is at least one argument provided by user.
if (args.Length < 1){
Console.WriteLine("Need at least one arg.");
return;
}Interesting thing is that Visual Studio Code has these little helpers that pop up at various times which state [Show fixes]. This one says, "invert if"[^]. If you click, it changes the code to:
if (args.Length >= 1){
return;
}
Console.WriteLine("Need at least one arg.");
return;Do you find that clearer? I don't. In my case, the if statement occurs at the top and if it is not fulfilled then the app exits. In that case there is no need to think about other code. Plus, the code that executes normally will not be wrapped in any outer if statement, instead it will simply following the if statement in a normal reading flow. Inverted Case In the inverted if then when there is at least one argument then all of your base code is now wrapped in the if statement and you have to think backwards. It's weird. AI you have failed me. :|
This is mostly useful if you've got multiple nested ifs and the indentation is getting obnoxious.
if (condition1)
{
Something1();
if (condition2)
{
Something2();
if (condition3)
{
Something3();
if (condition4)
{
Something4();
if (condition5)
{
Something(5);
}
else
{
return;
}
}
else
{
return;
}
}
else
{
return;
}
}
else
{
return;
}
}
else
{
return;
}flattens to:
if (!condition1)
{
return;
}
Something1();if (!condition2)
{
return;
}
Something2();if (!condition3)
{
return;
}
Something3();if (!condition4)
{
return;
}
Something4();if (!condition5)
{
return;
}
Something5();Did you ever see history portrayed as an old man with a wise brow and pulseless heart, weighing all things in the balance of reason? Is not rather the genius of history like an eternal, imploring maiden, full of fire, with a burning heart and flaming soul, humanly warm and humanly beautiful? --Zachris Topelius Training a telescope on one’s own belly button will only reveal lint. You like that? You go right on staring at it. I prefer looking at galaxies. -- Sarah Hoyt
-
This is mostly useful if you've got multiple nested ifs and the indentation is getting obnoxious.
if (condition1)
{
Something1();
if (condition2)
{
Something2();
if (condition3)
{
Something3();
if (condition4)
{
Something4();
if (condition5)
{
Something(5);
}
else
{
return;
}
}
else
{
return;
}
}
else
{
return;
}
}
else
{
return;
}
}
else
{
return;
}flattens to:
if (!condition1)
{
return;
}
Something1();if (!condition2)
{
return;
}
Something2();if (!condition3)
{
return;
}
Something3();if (!condition4)
{
return;
}
Something4();if (!condition5)
{
return;
}
Something5();Did you ever see history portrayed as an old man with a wise brow and pulseless heart, weighing all things in the balance of reason? Is not rather the genius of history like an eternal, imploring maiden, full of fire, with a burning heart and flaming soul, humanly warm and humanly beautiful? --Zachris Topelius Training a telescope on one’s own belly button will only reveal lint. You like that? You go right on staring at it. I prefer looking at galaxies. -- Sarah Hoyt
I think that the construct of having multiple 'if (condition) return' at the start of a function is meant to be part of the concept of testing preconditions are met before getting to doing the real 'meat' of the function. When I started, the general rule was that every block (function / if statement / loop / switch / etc) had exactly 'one point in, one point out' - leading to the rule that there was only one 'return' which was at the bottom of the function. There are two ways of reducing the nesting from code like
if (test1)
{
dosomething1();
if (test2)
{
dosomething2();
if (test3)
{
dosomething3();
/* etc */
}
else
return;
}
else
return;
}
else
return;One is saving a running success / fail status. Viz:
bool OK = test1;
if (OK)
{
dosomething1();
OK = test2;
}if (OK)
{
dosomething2();
OK = test3;
}if (OK)
{
dosomething3();
/* etc */
}return;
(This does preserve the 'one point in, one point out' rule but is slightly clunky - but I have used it many, many times) A slightly less clunky alternative to this pattern is ..
bool OK = test1;
if (OK)
dosomething1();OK &= test2;
if (OK)
dosomething2();OK &= test3;
if (OK)
dosomething3();/* etc */
return;
but I find this less obvious in its intent. The other alternative is to use short circuiting by making all of the
dosomething_n_()routines always returntrue. Viz:bool _ =
test1 && dosomething1()
&& test2 && dosomething2()
&& test3 && dosomething3()
&& etc;return;
As long as you comment it at the top to explain how it works in case the next developer isn't used to this idiom, then this last one is succinct, efficient and retains the 'one point in, one point out' design.
-
I think that the construct of having multiple 'if (condition) return' at the start of a function is meant to be part of the concept of testing preconditions are met before getting to doing the real 'meat' of the function. When I started, the general rule was that every block (function / if statement / loop / switch / etc) had exactly 'one point in, one point out' - leading to the rule that there was only one 'return' which was at the bottom of the function. There are two ways of reducing the nesting from code like
if (test1)
{
dosomething1();
if (test2)
{
dosomething2();
if (test3)
{
dosomething3();
/* etc */
}
else
return;
}
else
return;
}
else
return;One is saving a running success / fail status. Viz:
bool OK = test1;
if (OK)
{
dosomething1();
OK = test2;
}if (OK)
{
dosomething2();
OK = test3;
}if (OK)
{
dosomething3();
/* etc */
}return;
(This does preserve the 'one point in, one point out' rule but is slightly clunky - but I have used it many, many times) A slightly less clunky alternative to this pattern is ..
bool OK = test1;
if (OK)
dosomething1();OK &= test2;
if (OK)
dosomething2();OK &= test3;
if (OK)
dosomething3();/* etc */
return;
but I find this less obvious in its intent. The other alternative is to use short circuiting by making all of the
dosomething_n_()routines always returntrue. Viz:bool _ =
test1 && dosomething1()
&& test2 && dosomething2()
&& test3 && dosomething3()
&& etc;return;
As long as you comment it at the top to explain how it works in case the next developer isn't used to this idiom, then this last one is succinct, efficient and retains the 'one point in, one point out' design.
I have refactored code to eliminate this lamentable idiom. If the function is long, it usually needs to be broken up anyway. If it isn't, the idiom just introduces noise.
Robust Services Core | Software Techniques for Lemmings | Articles
-
I don't like the
args.Length < 1test - it's either going to be greater than or equal to1, or equal to0. Just for fun, you could also introduce some C# 8 into the mix, and support multiple commands:static void Main(string[] args)
{
if (args.Length == 0)
{
Console.WriteLine("Please provide 1 argument to indicate the command you want to run.\nUsage: getInfo ");
return;
}foreach (string arg in args) { Console.WriteLine(arg.ToLowerInvariant() switch { "os" => $"OS : {Environment.OSVersion}", "pwd" => $"The current directory is: {Environment.CurrentDirectory}", "cl" => $"Command line was: {Environment.CommandLine}", "sysdir" => $"System dir: {Environment.SystemDirectory}", "mname" => $"Machine name: {Environment.MachineName}", \_ => $"Unknown command: '{arg}'", }); }}
"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer
-
I prefer your version, but VS can get weird with it's hints. Sometimes it makes a suggestion, you let it implement it, and it immediately suggests something else and offers you the original code you wrote. For example:
StringBuilder sb = new StringBuilder(120);
Suggestion:
Use an implicit type:var sb = new StringBuilder(120);
Now it suggests
Use an explicit type instead of 'var':StringBuilder sb = new StringBuilder(120);
Which is what I prefer anyway ... :sigh:
"I have no idea what I did, but I'm taking full credit for it." - ThisOldTony "Common sense is so rare these days, it should be classified as a super power" - Random T-shirt AntiTwitter: @DalekDave is now a follower!
-
Whoa! Hold the phone. I hadn't really dived into the C# 8 stuff yet. So a switch supports an expression syntax now? That's clean.
Indeed. :) switch expression - C# reference | Microsoft Docs[^] What's new in C# 8.0 - C# Guide | Microsoft Docs[^]
"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer
-
This is not a true case of yoda conditionals or at least not in the sense I've heard the term. For me, yoda conditionals are written like:
if (1 >= args.Length)
{
Console.Write ("Not enough arguments");
return;
}The inversion refers to the order of terms in the if and the reason for it is that a construct like:
if (1 = args.Length)
(note the missing equal sign) will get you slapped with a big fat error message ('1' is not a l-value). The "normal" order might or might not produce a warning depending on compiler's whims. In your case it's just a case of compiler being annoying. Sometime I feel it's becoming almost like Clippy: "It seems you want to write an if statemenet. Do you need help with that?". Here is a random example taken straight out of some code:
assert (str);
while (*str > 0 && *str <= ' ')
str++;And it flags the while statement with an IntelliSense warning: "Dereferencing NULL pointer". Heck no! That's why I put the assert right before it.
Mircea
-
asserts( ) can be removed entirely based on compiler settings. That would explain the warning.
I know, but I didn't have NDEBUG defined in that configuration so assert was active. Don't get me wrong, I agree with having Intellisense looking over my shoulder and flagging potential issues but sometimes it gets annoying just like sometimes a coworker can be annoying when you do pair programming. And just as with a coworker, it feels good to bitch from time to time ;P
Mircea
