|
Maximilien wrote: They are hints.
I agree. Just suggestions. 
Maximilien wrote: In that case, it is not useful
Thanks for the confirmation. That's what I thought, but was wondering if I missed something.
Also, it really is just a matter of opinion on this one since either would work.
|
|
|
|
|
raddevus wrote: AI you have failed me.
Problem found. 
BTW... I am with you in this case.
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.
|
|
|
|
|
Nelek wrote: Problem found.
Nelek wrote: BTW... I am with you in this case.
Thanks. I thought all sane devs would be. The rest are not devs or are not sane or both. 
|
|
|
|
|
I'm curious if this error still pops up if you write more code below the if statement. I've never seen this before. My initial hunch is it might think you're doing the "if-good-else-bad" pattern since the only active code is in the if statement currently, and trying to move it around to the "early-exit/pre-condition" pattern which I personally do think is way cleaner. Or it may be attempting some pointless hyper-optimization? 
|
|
|
|
|
Jon McKee wrote: I'm curious if this error still pops up if you write more code below the if statement.
That's a very good question.
Here's the updated code that contains more code now. Note: this is simple code that is going in a book.
static void Main(string[] args)
{
if (args.Length < 1){
Console.WriteLine("Please provide 1 argument to indicate the command you want to run.\nUsage: getInfo <command-name>");
return;
}
switch (args[0].ToLower()){
case "os":{
Console.WriteLine($"OS : {Environment.OSVersion}");
break;
}
case "pwd":{
Console.WriteLine($"The current directory is: {Environment.CurrentDirectory}");
break;
}
case "cl":{
Console.WriteLine($"Command line was: {Environment.CommandLine}");
break;
}
case "sysdir":{
Console.WriteLine($"System dir: {Environment.SystemDirectory}");
break;
}
case "mname":{
Console.WriteLine($"Machine name: {Environment.MachineName}");
break;
}
}
}
Here's a snapshot of the invert if[^] that still shows up in VSC as a hint.
Here's the code you get if you invert now:
static void Main(string[] args)
{
if (args.Length >= 1)
{
switch (args[0].ToLower())
{
case "os":
{
Console.WriteLine($"OS : {Environment.OSVersion}");
break;
}
case "pwd":
{
Console.WriteLine($"The current directory is: {Environment.CurrentDirectory}");
break;
}
case "cl":
{
Console.WriteLine($"Command line was: {Environment.CommandLine}");
break;
}
case "sysdir":
{
Console.WriteLine($"System dir: {Environment.SystemDirectory}");
break;
}
case "mname":
{
Console.WriteLine($"Machine name: {Environment.MachineName}");
break;
}
}
}
else
{
Console.WriteLine("Please provide 1 argument to indicate the command you want to run.\nUsage: getInfo <command-name>");
return;
}
}
|
|
|
|
|
That's so bizarre. It seems like the purpose of this rule (from some short googling) is to reduce conditional nesting and/or cyclomatic complexity. But in this example it actually increases nesting by changing away from an early-exit guard clause and doesn't change the CC at all. It's like it's doing the exact opposite of what it should be doing
|
|
|
|
|
Jon McKee wrote: seems like the purpose of this rule (from some short googling) is to reduce conditional nesting and/or cyclomatic complexity.
Thanks for researching that and confirming that you find it odd too. And for putting some data behind it. I really thought it was strange but hadn't defined why very well.
Great info.
|
|
|
|
|
I don't like the args.Length < 1 test - it's either going to be greater than or equal to 1, or equal to 0.
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 <command-name>");
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
|
|
|
|
|
Richard Deeming wrote: I don't like the args.Length < 1 test
Yeah, admittedly this is just shorthand for "I only want to know if it is 0" otherwise keep on running. I talk about that in relation to the code however -- since this is really just part of a beginning code sample that is part of a larger context of explaining how to get arguments from command-line. I talk about this being a cheat that will simply ignore any arguments that are after the first one.
This is all part of a book (Programming Linux With .NET Core) and part of the beginning stages of teaching an early beginner.
The foreach code is a really good example of things that will come in later explanations and is much more elegant. Thanks
|
|
|
|
|
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.
|
|
|
|
|
|
That's a tool, not even a suggestion. And tools might be used when they are useful.
So: when?
E.g. you decided to add an else clause, and you prefer the order the other way round.
E.g. you have both the normal and the else clause, and decide the other way round looks better.
Then, the new code is just a mouse click away.
A tool. To be used when appropriate.
Not:
Here's a hammer. Now you have to use it, even when there's no nail.
Oh sanctissimi Wilhelmus, Theodorus, et Fredericus!
|
|
|
|
|
Bernhard Hiller wrote: Here's a hammer. Now you have to use it, even when there's no nail.
But if you only have a hammer... would not come the moment when all what you see is a nail?
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 agree. It's just that any time a tool attempts to have some "intelligence" about it then you begin to think it may have more context to the situation. But, this probably really is more like pop up code snippets and you take them or leave them. Also, because it has the initial pop up that states, "Show fixes (Ctrl+.)" it leads you to believe that something is wrong.
|
|
|
|
|
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.
|
|
|
|
|
I agree! Thanks
|
|
|
|
|
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 <usual> and <rare> 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.
|
|
|
|
|
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. 
|
|
|
|
|
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 ...
"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. 
|
|
|
|
|
Hello David, shall we play a game?  
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 that too. var was meant for implicit typing of anonymous-type variables; not to undermine expressive code 
|
|
|
|
|
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();
}
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();
}
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();
return; but I find this less obvious in its intent.
The other alternative is to use short circuiting by making all of the dosomethingn() routines always return true. 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.
modified 11-Aug-20 5:27am.
|
|
|
|
|
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.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
