Вопрос

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.

Лицензировано под: CC-BY-SA с атрибуция
Не связан с StackOverflow
scroll top