Cannot throw exception without getting the error: "Not all code paths return a value"

347 Views Asked by At

Im currently trying to make a new function inside of a class (in C#) and inside I have

public static Planet searchByName(String nameIn)
    {
        for (int i = 0; i < planetList.Count; i++)
        {
            Planet returnPlanet = null;
            if (planetList.ElementAt(i).name == nameIn)
            {
                returnPlanet = planetList.ElementAt(i);
            }
            if (returnPlanet == null)
            {
                throw new System.ArgumentException(String.Format("Planet with name {0} does not exist", nameIn));
            }
            return returnPlanet;
        }
    }

Im not sure how to throw the Exception and get it to return a value (or at least get rid of the current error)

Edit: Ah, I see, as many of you pointed out, the loop I have has a possibility of not running (although in my code it would always run, but I doubt the Compiler would know that)

3

There are 3 best solutions below

1
Sumner Evans On

If your for loop condition (i < planetList.Count) is false, the for loop won't be run. You need to return something if the for isn't run.

public static Planet searchByName(String nameIn)
{
    for (int i = 0; i < planetList.Count; i++)
    {
        . . .
    }

    return null;
}
3
tbridge On

What would happen if the planetList had no members? The loop would never enter and nothing would be returned.

You need to return something if this is the case. Simply returning null would work:

public static Planet searchByName(String nameIn)
{
    for (int i = 0; i < planetList.Count; i++)
    {
        Planet returnPlanet = null;
        if (planetList.ElementAt(i).name == nameIn)
        {
            returnPlanet = planetList.ElementAt(i);
        }
        if (returnPlanet == null)
        {
            throw new System.ArgumentException(String.Format("Planet with name {0} does not exist", nameIn));
        }
        return returnPlanet;
    }
    return null;
}

However, if you look closer at your code you'll find that on the first iteration the planet will either be found or an exception will be thrown, those are the only two options. You should only throw an exception if the entire list has been checked, which would looks like this:

public static Planet searchByName(String nameIn)
{
    for (int i = 0; i < planetList.Count; i++)
    {
        Planet planet = planetList.ElementAt(i);
        if (nameIn == planet.name)
            return planet;
    }
    throw new System.ArgumentException(String.Format("Planet with name {0} does not exist", nameIn));
}

You could also achieve the same goal with LINQ, which looks much nicer:

public static Planet searchByName(string name)
{
    return planetList.FirstOrDefault(planet => planet.name == name);
}
0
epalm On

You're looking for a Planet with a name field that matches nameIn.

The pseudocode for this would be something like

for every planet in the list of planets
    if planet's name == nameIn
        return plant
after the for loop, if no matching planet was found, throw an exception

Here's your code following my pseudocode

public static Planet searchByName(String nameIn)
{
    for (int i = 0; i < planetList.Count; i++)
    {
        if (planetList.ElementAt(i).name == nameIn)
        {
            return planetList.ElementAt(i);
        }
    }
    throw new System.ArgumentException(String.Format("Planet with name {0} does not exist", nameIn));
}

Here's how I would write it with a for-each loop:

public static Planet searchByName(String nameIn)
{
    for (var planet in planetList)
    {
        if (planet.name == nameIn)
        {
            return planet;
        }
    }
    throw new System.ArgumentException(String.Format("Planet with name {0} does not exist", nameIn));
}

Here's how I would write it with LINQ:

public static Planet searchByName(String nameIn)
{
    Planet planet = planetList.FirstOrDefault(p => p.name == nameIn);
    if (planet != null)
    {
        return planet;
    }
    else
    {
        throw new System.ArgumentException(String.Format("Planet with name {0} does not exist", nameIn))
    }
}