C# "Not all code paths return a value", but they do, don't they?

Question

I understand what this error means. I've read through the other related questions and posts. From what I can tell I've addressed the issue, but am still getting the error. Does anyone see something I don't?

    public static IWebElement RetrieveObject(IWebDriver driver, string page, string pageObject)
    {
        int rCnt = 0;

        string TablePath      = "C:\\Automation Projects\\Tables\\";
        string ObjectRepPath  = TablePath + "ObjectRepository.xlsx";
        string objpage        = " ";
        string objelement     = " ";
        string objelementType = " ";
        string objlocator     = " ";

        IWebElement locator = null;

        Application xlApp;
        Workbook xlWorkBook;
        Worksheet xlWorkSheet;
        Range range;

        xlApp       = new Application();
        xlWorkBook  = xlApp.Workbooks.Open(ObjectRepPath);
        xlWorkSheet = (Worksheet)xlWorkBook.Worksheets.get_Item(1);

        range = xlWorkSheet.UsedRange;

        for (rCnt = 1; rCnt <= range.Rows.Count; rCnt++)
        {
                    objpage        = (string)(range.Cells[rCnt, 1] as Range).Value2;
                    objelement     = (string)(range.Cells[rCnt, 2] as Range).Value2;
                    objelementType = (string)(range.Cells[rCnt, 3] as Range).Value2;
                    objlocator     = (string)(range.Cells[rCnt, 4] as Range).Value2;


            if (objpage == page && objelement == pageObject)
            {
                if (objelementType == "id")
                {
                    locator = driver.FindElement(By.Id(objlocator));
                    return locator;
                }
                else if (objelementType == "name")
                {
                    locator = driver.FindElement(By.Name(objlocator));
                    return locator;
                }
                else if (objelementType == "xpath")
                {
                    locator = driver.FindElement(By.XPath(objlocator));
                    return locator;
                }
                else
                {
                    return locator;
                }
            }
        }
    }

I'm returning the iWebElement 'locator' for each possible path. I'm sure I'm missing something simple, but I've been staring at it too long and can't see it.

what if there are no rows at all? what if there are no rows that match `page` / `pageObject`? — Marc Gravell, Jul 24 '18 at 15:27
`I'm returning... for each possible path.` Except the big one. What happens if you never even enter the loop? Which path does the function take? — , Jul 24 '18 at 15:28
Out of curiosity, where did you learn to structure code that way? I've seen it before and just want to know where it comes from. — Yuriy Faktorovich, Jul 24 '18 at 15:29
There is no guarantee of going into loop or if statement so you need to add return statement which can be accessible for every possible condition. — Murat Tüfekçi, Jul 24 '18 at 15:30
Nevermind, I figured it out. I removed all the return statements and just added one just outside of the 'for' loop. Now it's happy. — HeadlyvonNoggin, Jul 24 '18 at 15:32
@Yuriy - Is my code structure weird? I'm self-taught, so I'm sure I'm breaking all kinds of rules and best practices structure wise. — HeadlyvonNoggin, Jul 24 '18 at 15:34
@HeadlyvonNoggin yes, you're declaring all your variables at the beginning of the method instead of where they are being used. It ends up with unused items being declared when you refactor methods. Also having to scroll up and down to figure out what something is. — Yuriy Faktorovich, Jul 24 '18 at 15:39
@HeadlyvonNoggin here's a slightly "cleaned up" (although that is hugely subjective) version for comparison: https://gist.github.com/mgravell/4bd1d3861740a9125accb644798bbb88 - but: it isn't *wrong* - just stylistically unusual — Marc Gravell, Jul 24 '18 at 15:40
@Yuriy ... oh, yeah. I can see that. I do it this way because it seems cleaner and more organized. And hovering over the variable to see what it is if need be generally keeps me from having to scroll. — HeadlyvonNoggin, Jul 24 '18 at 15:42
@HeadlyvonNoggin do you know what keeps me from having to scroll? having the method fit in 20ish lines :) — Marc Gravell, Jul 24 '18 at 15:43
@Marc - I like it. I'm always self conscious about how I structure things. I have no formal training and am sure I do things "weird" all the time. It usually accomplishes what I set out to do, but I'm sure comes off as 'clunky' to more seasoned coders. — HeadlyvonNoggin, Jul 24 '18 at 15:47
@MarcGravell I will call that the Train Pattern. Take a long method, and split it up into a bunch of methods all calling each other in a nested pattern. You can use ReSharper to speed up the process. Just count off 20 lines then Refactor -> Extract -> Extract Method for the rest of the code recursively. — Yuriy Faktorovich, Jul 24 '18 at 15:50

Sean Stayns · Answer 1 · 2018-07-24T15:33:51.520

They don't. There is one question you should ask to yourself: What, if you have no row?

You have to add a return statement after the for-loop.

public static IWebElement RetrieveObject(IWebDriver driver, string page, string pageObject)
{
    int rCnt = 0;

    string TablePath      = "C:\\Automation Projects\\Tables\\";
    string ObjectRepPath  = TablePath + "ObjectRepository.xlsx";
    string objpage        = " ";
    string objelement     = " ";
    string objelementType = " ";
    string objlocator     = " ";

    IWebElement locator = null;

    Application xlApp;
    Workbook xlWorkBook;
    Worksheet xlWorkSheet;
    Range range;

    xlApp       = new Application();
    xlWorkBook  = xlApp.Workbooks.Open(ObjectRepPath);
    xlWorkSheet = (Worksheet)xlWorkBook.Worksheets.get_Item(1);

    range = xlWorkSheet.UsedRange;

    for (rCnt = 1; rCnt <= range.Rows.Count; rCnt++)
    {
                objpage        = (string)(range.Cells[rCnt, 1] as Range).Value2;
                objelement     = (string)(range.Cells[rCnt, 2] as Range).Value2;
                objelementType = (string)(range.Cells[rCnt, 3] as Range).Value2;
                objlocator     = (string)(range.Cells[rCnt, 4] as Range).Value2;


        if (objpage == page && objelement == pageObject)
        {
            if (objelementType == "id")
            {
                locator = driver.FindElement(By.Id(objlocator));
                return locator;
            }
            else if (objelementType == "name")
            {
                locator = driver.FindElement(By.Name(objlocator));
                return locator;
            }
            else if (objelementType == "xpath")
            {
                locator = driver.FindElement(By.XPath(objlocator)); 
                return locator;
            }
            else
            {
                return locator;
            }
        }
    }
    return locator; // <---------------- !!
}

Or alternatively you could have thrown an exception here, much like I put in my answer — Horkrine, Jul 24 '18 at 15:30
Exception is probably the most suitable option, as OP didn't even consider that a value might not be returned, indicating an exceptional circumstance. — cwharris, Jul 24 '18 at 16:05

C# "Not all code paths return a value", but they do, don't they?

1 Answers1