Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why is i++ "unreachable code"?

EDIT

I'm leaving this up here, even though it makes me look stupid, because it's a subtle bug that can bite you if you're working late at night and not paying attention. Kudos to Visual Studio for having such an intelligent parser.

Basically I missed that I have nested loops, so the continue statement is essentially worthless here because it is continuing the foreach loop, not the for loop.

ORIGINAL QUESTION

I'm running a search on a workbook, looking for the sheet that matches all the string search criteria. In the Visual Studio editor, the i++ is underlined as "unreachable code".

/// <summary>
/// Finds the first sheet that has cells that match all the criteria.
/// </summary>
/// <param name="wb"></param>
/// <param name="searches"></param>
/// <returns></returns>
public static ISheet FindSheet( this IWorkbook wb, params string[] searches )
{
    if( null == wb || null == searches )
        return null;

    for( int i = 0; i < wb.NumberOfSheets; i++ )
    {
        var sheet = wb.GetSheetAt( i );
        foreach( var search in searches )
        {
            var cell = sheet.FindCell( search );
            if( null == cell )
                continue;
        }

        return sheet;
    }

    return null;
}

I would think that the continue statement has a clear meaning here: "If any of the search criteria return a null cell, continue to the next iteration. Otherwise, just return the sheet found in this iteration."

CORRECTED CODE WITH NO CONTINUE STATEMENT

/// <summary>
/// Finds the first sheet that has cells that match all the criteria.
/// </summary>
/// <param name="wb"></param>
/// <param name="searches"></param>
/// <returns></returns>
public static ISheet FindSheet( this IWorkbook wb, params string[] searches )
{
    if( null == wb || null == searches )
        return null;

    for( int i = 0; i < wb.NumberOfSheets; i++ )
    {
        var sheet = wb.GetSheetAt( i );
        if( searches.All( s => null != sheet.FindCell( s ) ) )
            return sheet;
    }

    return null;
}
like image 813
Bruce Pierson Avatar asked Feb 13 '23 10:02

Bruce Pierson


2 Answers

Here, your for loop in completely useless. After the first iteration, it will just return no matter what. Sure your foreach loop has a continue statement, but this only applies to the foreach loop. This is why your code is unreachable.

Because of this, only the first sheet will be analysed. If this is what you want, you can simply remove the for loop and target the Worksheet at index 0, or you need to reaarange your loop.

like image 153
gretro Avatar answered Feb 16 '23 01:02

gretro


Eric Lippert has an SO answer and a blog post on what you can do when you want to continue an outer loop when you're in an inner loop.

The technique he has marked as "awesome" is to get rid of all of the loops when it's possible to do so, and indeed I believe it is possible here:

public static ISheet FindSheet( this IWorkbook wb, params string[] searches )
{
    if( null == wb || null == searches ) { return null; }

    return wb.FirstOrDefault(sh => searches.All(sr => sh.FindCell(sr) != null));
}
like image 44
JLRishe Avatar answered Feb 16 '23 03:02

JLRishe