Is this a mistake in/by Resharper?
-
30-06-2021 - |
Вопрос
I had this code:
string[] args = Environment.GetCommandLineArgs();
bool grabNext;
foreach (string arg in args)
{
if (arg == "-AA")
{
grabNext = true;
}
if (grabNext)
{
incomingPlatypusID = arg;
}
}
...and Resharper suggested declaring "bool grabNext" in inner scope, so when I allowed it to, the code became:
string[] args = Environment.GetCommandLineArgs();
foreach (string arg in args)
{
if (arg == "-AA") ;
bool grabNext;
{
grabNext = true;
}
if (grabNext)
{
incomingPlatypusID = arg;
}
}
...then, of course, I got an err msg, namely, "Empty control statement body"
UPDATE
Actually, what I think I need is the following, as the "-AA" is my indication to grab the next arg:
foreach (string arg in args)
{
if (arg == "-AA")
{
grabNext = true;
continue;
}
if (grabNext)
{
PlatypusID = arg;
break;
}
}
UPDATE 2
With this code Resharper does not complain:
private void AutoProvMainForm_Load(object sender, EventArgs e)
{
string[] args = Environment.GetCommandLineArgs();
bool grabNext = false;
foreach (string arg in args)
{
if (arg == "-AA")
{
grabNext = true;
continue;
}
if (grabNext)
{
PlatypusID = arg;
break;
}
}
}
Решение
What Reshaper wants is something like this:
foreach (string arg in args)
{
bool grabNext = (arg == "-AA");
if (grabNext)
{
incomingPlatypusID = arg;
// probably better break now:
break;
}
}
Note that the code always sets incomingPlatypusID
to "-AA"
(if args contains it).
Increases readability and puts grabNext
into the scope where it belongs to since it is used only there.
Here are more infos: Scope of variables in C#
Excerpt:
In C#, it's better to place them at the scope where they are needed. This has a few benefits, including:
- You reduce the risk of error from reusing a variable inappropriately, especially during long term maintenance
- You are keeping the variable constrained within that scope, which eases refactoring
Note that the same statement could be even more readable using Linq:
incomingPlatypusID = args.FirstOrDefault(a => a == "-AA");
Edit: Update according to your modified question. Use a for-loop
instead if you want to use indexer.
for( int i = 0; i < args.Length; i++ )
{
if( args[i] == "-AA" && i+1 < args.Length )
{
incomingPlatypusID = args[ i+1 ];
break;
}
}
Другие советы
with grabNext declared outside of the for each you could have the previous arg value of grabNext since it is only overridden if arg == "-AA".
GrabNext could be true from the previous iteration.