Finding an item in a List<T>

Question

I have the follow example:

public class Commands
{
    public int ID { get; set; }
    public List<string> Alias { get; set; }
}

public class UserAccess
{
    public int AccessID { get; set; }
    // other stuff not needed for the question
    public List<Commands> AllowedCommands { get; set; }
}

Now I wanted to implement on the UserAccess a way to return the command ID or NULL if no Alias were found on the list, see a dirty example of what I am saying below HasCommand:

public class UserAccess
{
    public ID { get; set; }
    // other stuff not needed for the question
    public List<Commands> AllowedCommands { get; set; }

    public Commands HasCommand(string cmd)
    {
        foreach (Commands item in this.AllowedCommands)
        {
            if (item.Alias.Find(x => string.Equals(x, cmd, StringComparison.OrdinalIgnoreCase)) != null)
                return item;
        }
        return null;
    }
}

• My question is what would be the most efficient way to run or implement the HasCommand method ?

• Or is there a better way to implement it into the UserAccess ?

Answer 1

Can be shortened a little bit

public Commands HasCommand(string cmd)
{
    return AllowedCommands.FirstOrDefault(c => c.Alias.Contains(cmd, StringComparer.OrdinalIgnoreCase));

}

but it's pretty much the same thing.

Answer 2

public Commands HasCommand(string cmd)
    {
        return this.AllowedCommands.FirstOrDefault(item => item.Alias.Find(x => string.Equals(x, cmd, StringComparison.OrdinalIgnoreCase)) != null);
    }

You do not need to use Where + FirstOrDefault. The FirstOfDefault can have condition.

Answer 3

Also, 3 suggestions for further improvement:

(1) I would encourage the use of IEnumerable instead of List, if possible.
(2) I would call "Commands" just "Command".
(3) I would make all commands be able to be easily referenced via a class like this:

public class Command {
    public Command(int id, IEnumerable<string> aliases) {
        Id = id;
        Aliases = alias;
    }

    public int Id { get; set; }         
    public IEnumerable<string> Aliases { get; set; }  
}

public class Commands {
    public static readonly Command CommandNameHere1(yourIdHere1, yourAliasesHere1);
    public static readonly Command CommandNameHere2(yourIdHere2, yourAliasesHere2);
    //etc.
}

Answer 4

Assuming that by "efficient", you mean fast, anytime you are looking up a string in a collection of strings, and that collection is likely to contain more than a few entries, you should always use a hash lookup. Doing a simple scan of the list takes exponential time as the count of items goes up, while the count has little effect on a hash lookup. In .NET, this has traditionally been handled by the Dictionary class, which is commonly used to index a collection of objects with a key (which is often a string). However, the value can't be null, and this led to passing the same string in as both the key and value - rather ugly. Finally, .NET 4 provided HashSet, which you should use for such a case of only having a key and no value.

In your case, you have the (not uncommon) situation of needing a case-insensitive compare. The common solution for this is to lower-case the string keys when adding them to the dictionary (or HashSet). This tiny overhead on add is vastly outweighed by the savings on lookups, since all programmers should know and understand that case-insensitive compares are vastly slower than case-sensitive, especially with Unicode - the CPU can't just do a block compare of data, but must check each pair of characters specially (even using a table look-up, this is vastly slower).

If your Alias names can be in lower case, change them from List to HashSet. If not, use Dictionary where the key is added as lower case, and the value is the (mixed-case) Alias string. Assuming the use of Dictionary, your code would become:

public Commands HasCommand(string cmd)
{
    foreach (Commands item in AllowedCommands)
    {
        if (item.Alias.ContainsKey(cmd))
            return item;
    }
    return null;
}

Finally, also on the subject of performance, using LINQ is almost always going to result in slower performance - somewhere between a little slower and a lot slower, depending upon the situation. It does make nice, compact source for simple things, and I use it quite a bit myself, but if you're certain that performance is an issue for a piece of a code, you probably shouldn't use it (unless it's PLINQ, of course).

So, if you want as few lines of code as possible, use the other answer posted here. If you want speed, use mine.

It almost goes without saying, but when you're worried about the performance of some small chunk of code like this, just wrap it in a for loop and repeat it until it takes 5-10 seconds to execute - just add orders of magnitude as needed, whether it's 1,000 or 1,000,000 reps, and time it with System.Diagnostics.Stopwatch. Try alternative logic, and repeat the test. The 5-10 seconds is a minimum designed to mask the fluctuations caused by a managed environment and other stuff executing on the same machine (you should obviously also avoid running other apps during the test). Of course, for overall performance testing of a complicated application, a performance analyzer tool would be recommended.