Checking a value in a string array in C# [closed]

Question

I want to see if there is a cleaner way to do this logic. I have a working sample:

Same fiddle.net

public static Boolean passwordchecker(string userName)
    {
        string[] group = new string[] { "a", "b" };
        if (group[0] == userName || group[1] == userName)
        {
            return true;
        }
        else
        {
            return false;
        }
    }

Answer 1

Your code has three issues:

1. The name passwordchecker: good method names are almost always a verb because they do things. I’d use CheckPassword (the capitalisation follows standard C# naming conventions). And, while we’re talking about names: the variable name group does not describe what the variable does, and, as a consequence, it took me unnecessary time to understand what your code is supposed to do. And, lastly, the parameter you call userName isn’t a user name; it’s a password, isn’t it? If so, you need to fix it. If it’s correct, you need to fix your method name because apparently it checks user names, not passwords.

2. The code uses the anti-pattern if (expr) return true; else return false;. This simply never makes sense, and it never makes for clearer code when compared to the alternative: return expr;. In fact, outside of direct variable initialisation, there’s virtually no use for the literals true and false.

3. There’s an array, but the code uses its values separately. As it stands, there is no reason for this to be an array (rather than separate variables), and it’s (very slightly) error-prone since a future maintainer might be tempted to extend the array, but forget to adjust its usage. AlanT’s answer shows one way of checking whether a value can be found inside an array. Personally, I’d be tempted to use the System.Linq.Enumerable.Contains method instead, because it doesn’t require using a predicate:

   public static bool CheckPassword(string password)
   {
       var passwords = new[] { "a", "b" };
       return passwords.Contains(password);
   }
   

   This requires using System.Linq;. Without that, you’ll have to cast the array explicitly to IList<string> because arrays only implement the IList<T> interface privately.

Furthermore, as shown in AlanT’s answer, I’d be tempted to make passwords a private readonly static variable rather than a local variable.

Answer 2

Some of the naming seems off - it is a password checker but you are passing in and comparing a userName - but if you want a linq-ish solution you might try

private readonly static string[] Group = { "a", "b"};

private static bool PasswordChecker(string password)
{
    return Group.Any(g => g==password);
}

Notes:
I would generally make names of methods PascalCased in C# rather than all lower case.

Answer 3

Definitely some shorter/more performant ways to write that:

private static readonly string[] group = new string[] { "a", "b" };

public static bool passwordchecker(string userName) => group[0] == userName || group[1] == userName;

for example.

Answer 4

I'd suggest something closer to this:

A method's name should describe what it's doing.
When a method returns a bool, it's often best to start the method name with a verb like "is" so that it asks a question.

If statements should be used sparingly.
Else statements should be used even less.

    public static bool IsUserNameValid(string userName) =>
        new List<string> { "a", "b" }
        .Any(value => string.Equals(value, userName, StringComparison.InvariantCultureIgnoreCase));