Searching for an object in a list returning boolean and referencing variable

Question

My method searches through an list of accounts which have the property "Name".

class Accounts
{
    static internal List<Account> accounts = new List<Account>();

    static internal bool findAccount(string name, ref int account)
    {
        for (int i = 0; i < accounts.Count; i++)
        {
            if (accounts[i].Name == name)
            {
                account = i;
                return true;
            }
        }
        return false;
    }
}

The variable it is referencing is in the main form:

public partial class Lodge : Form
{
    public Lodge()
    {
        InitializeComponent();
    }

    private void btn_lodge_Click(object sender, EventArgs e)
    {
        int account = -1;
        bool found = Accounts.findAccount(tBox_wName.Text, ref account);
        if (found && account >= 0)
        {
            decimal lodge = Convert.ToDecimal(tBox_tAmou.Text);
            Accounts.accounts[account].lodge(lodge);
            lbl_stats.Text = "Successfully lodged. Balance is now: " + Accounts.accounts[account].Balance;
        }
        else
        {
            lbl_stats.Text = "Account not found.";
        }
    }

I am still learning C#, so I am still learning the most efficient ways of doing things. This code works as it should, but I feel like the fact that I am returning a boolean and referencing the account variable seems wrong.

The program is basically lodging money to an account (simply an object containing the person's name, etc..). So there is a static Accounts class and a non-static class 'Account' for each account. I have made a static class because these functions are to be shared across multiple forms.

Should I remove the boolean and simply have it return the account to the account variable? What would be the most accepted way by a more experience programmer?

Answer 1

The List type provides a List-specific Exists method which will return a boolean telling you whether a value that meets the specified predicate exists in the list:

var found = Accounts.Exists(a => a.Name == tBox_wName.Text);

Instead of using that though, just use Find(predicate) to return the first value for which the predicate returns true (or the type's default value if not found which is null for reference types).

Array has similar methods.

If you're using any IEnumerable you can use .Any() or FirstOrDefault().

If I was rolling this by hand (which in your case I don't believe is necessary), I'd prefer the following pattern:

Account account;
if(TryFindAccount(accounts, out account))
{
    ....
}

or

Account account;
if(TryFindAccountByName(accounts, name, out account))
{
    ....
}

Unless you really needed to track by index across layers (for which I don't usually find a compelling reason to do so), I'd deal directly with the account as soon as possible.

The above implementation also avoids checks against magic values, like checking if account is null, or in your example, -1. If you can, avoid attributing special meaning to certain values.

Answer 2

With find the most common way is to return the index (or iterator) of the found value and -1 if the value is not found. This is exactly what List.FindIndex does. However C# has a feature called Nullable types which you can use:

static internal int? findAccount(string name)
{
    for (int i = 0; i < accounts.Count; i++)
    {
        if (accounts[i].Name == name)
        {
            return i;

        }
    }
    return null;
}

private void btn_lodge_Click(object sender, EventArgs e)
{

    int? account = Accounts.findAccount(tBox_wName.Text);
    if (account.HasValue)
    {
        decimal lodge = Convert.ToDecimal(tBox_tAmou.Text);
        Accounts.accounts[account.Value].lodge(lodge);
        lbl_stats.Text = "Successfully lodged. Balance is now: " + Accounts.accounts[account.Value].Balance;
    }
    else
    {
        lbl_stats.Text = "Account not found.";
    }
}

Here int? means that the value may or may not have a value which you can check with HasValue.

However the way you use it if by then getting the account immediately if you found it.

So why not return the account directly?

static internal Account findAccount(string name)
{
    for (int i = 0; i < accounts.Count; i++)
    {
        if (accounts[i].Name == name)
        {
            return accounts[i];

        }
    }
    return null;
}


private void btn_lodge_Click(object sender, EventArgs e)
{

    Account account = Accounts.findAccount(tBox_wName.Text);
    if (account!=null)
    {
        decimal lodge = Convert.ToDecimal(tBox_tAmou.Text);
        account.lodge(lodge);
        lbl_stats.Text = "Successfully lodged. Balance is now: " + account.Balance;
    }
    else
    {
        lbl_stats.Text = "Account not found.";
    }
}

This lets you later use a Dictionary<string, Account> to store the accounts.