Not all code paths return a value (bools)

Question

I'm in the process of creating a messaging service application for experience. However, I've stumbled across an error I've encountered before. Here is my method:

bool userExists(string pcName)
    {
        string[] files = Directory.GetFiles(usersFile);

        foreach (string fileName in files)
        {
            if (fileName == pcName)
            {
                return true;
            }
            else
            {
                return false;
            }
        }
    }

As you can see, this method returns a yes or no answer on if the user's txt file exists in the folder. Now, to me, all code paths return a value. It makes sense, obviously the compiler doesn't think the same because I get an error on "bool userExists" complaining that not all code paths return a value. How can I fix this issue? Thanks!

Answer 1

To start with, why use GetFiles? Use EnumerateFiles instead.

bool UserExists(string pcName)
{
    return Directory.EnumerateFiles(this.usersFile).FirstOrDefault(f => f == pcName);
}

This code achieves the functionality equivalent to your code.

However, you may actually want,

bool UserExists(string pcName)
{
    return Directory.EnumerateFiles(this.usersFile).Any(f => f == pcName);
}

Enumeration will begin immediately without the needless creation of an intermediate array. If a match is found early needless processing and reading from disk will be avoided.

---

You can avoid your "Not all code paths return a value" message by doing

bool userExists(string pcName)
{
    string[] files = Directory.GetFiles(usersFile);

    return files.Length > 0 && files[0] == pcName;
}

if you want to check only the first file. This handles the case where files is empty. Although, I suspect you actually want,

bool userExists(string pcName)
{
    string[] files = Directory.GetFiles(usersFile);

    foreach (string fileName in files)
    {
        if (fileName == pcName)
        {
            return true;
        }            
    }

    return false;
}

to check for any file.

Answer 2

What if the files was empty? so you can do this:

string[] files = Directory.GetFiles(usersFile);

foreach (string fileName in files)
{
    if (fileName == pcName)
    {
        return true;
    }
}

return false;

Answer 3

What happens if for example, the list files is empty?
The compiler reach the end of the function without hitting a return statement, that's what it tells you.

The good way to write your code is:

bool userExists(string pcName)
{
    string[] files = Directory.GetFiles(usersFile);

    foreach (string fileName in files)
    {
        if (fileName == pcName)
        {
            return true;
        }
    }
    return false;
}

And it could be the moment to become familiar with Linq:

bool userExists(string pcName)
{
    return Directory.GetFiles(usersFile)
                    .Any(file => file == pcName);
}

Answer 4

What if your files has no file?

Your code will skip foreach statement but it wouldn't return anything because out of your foreach statement, you don't have any return statement.

string[] files = Directory.GetFiles(usersFile);

foreach (string fileName in files)
{
    if (fileName == pcName)
    {
        return true;
    }
}

return false or true;

Or there is other Linq way to solve;

return Directory.GetFiles(usersFile)
                .Where(fileName => fileName == pcName)
                .Any();

Answer 5

The issue is that your array of 'files' could be empty. If that is the case, you the boolean return statements would never be hit. Put a final 'return false;' after the for...each loop and you won't get any more errors.

That being said, your fundamental logic is flawed. By returning false in your else statement, the loop will never continue (assuming there is more than one file).

Answer 6

bool userExists(string pcName)
{
    return Directory
        .GetFiles(usersFile)
        .Any(fileName => fileName == pcName);
}