CODEWITHSUNDEEP
c#c#-3.0extension-methods
Nick Allen
Nick Allen

Reputation: 12220

My first extension method, could it be written better?

Because this is my first attempt at an extension method that seems quite useful to me, I just want to make sure I'm going down the right route 

 public static bool EqualsAny(this string s, string[] tokens, StringComparison comparisonType)
    {
        foreach (string token in tokens)
        {
            if (s.Equals(token, comparisonType))
            {
                return true;
            }
        }

        return false;
    }

Called by

if (queryString["secure"].EqualsAny(new string[] {"true","1"}, StringComparison.InvariantCultureIgnoreCase))
{
    parameters.Protocol = Protocol.https;
}

EDIT: Some excellent suggestions coming through, exactly the sort of thing I was looking for. Thanks

EDIT:

I have decided on the following implementation

public static bool EqualsAny(this string s, StringComparison comparisonType, params string[] tokens)
{
    // for the scenario it is more suitable for the code to continue
    if (s == null) return false;

    return tokens.Any(x => s.Equals(x, comparisonType));
}

public static bool EqualsAny(this string s, params string[] tokens)
{
    return EqualsAny(s, StringComparison.OrdinalIgnoreCase, tokens);
}

I preferred using params over IEnumerable because it simplified the calling code

if (queryString["secure"].EqualsAny("true","1"))
{
    parameters.Protocol = Protocol.https;
}

A far cry on the previous

if (queryString["secure"] != null)
{
    if (queryString["secure"] == "true" || queryString["secure"] == "1")
    {
        parameters.Protocal = Protocal.https;
    }
}

Thank you again!

Upvotes: 6

Views: 437

Answers (6)

Joel Coehoorn
Joel Coehoorn

Reputation: 415690

Yes! First, you need to check s for null. Also, let it accept any IEnumerable<string> for tokens rather than just an array, and then use other linq operators to do the check:

public static bool EqualsAny(this string s, IEnumerable<string> tokens, StringComparison comparisonType)
{
    if (s== null) return false;
    return tokens.Any(t => s.Equals(t, comparisonType));
}

Thinking about how to handle a null value for s, there's a third option no one's used yet:

 public static bool EqualsAny(this string s, IEnumerable<string> tokens, StringComparison comparisonType)
{
    if (s== null) return tokens.Any(t => t == null);
    return tokens.Any(t => s.Equals(t, comparisonType));
}

Finally, regarding your chosen implementation: if you're going to have overloads, you might as well have IEnumerable overloads as well, and have your params code call those.

Upvotes: 7

dfa
dfa

Reputation: 116324

in order to simplify usage of EqualsAny you could use varargs and default strategy for StringComparison:

public static bool EqualsAny(this string s, params string[] tokens) {
    return EqualsAny(s, StringComparison.InvariantCultureIgnoreCase, tokens);
}

public static bool EqualsAny(this string s, 
                             StringComparison stringComparison, 
                             params string[] tokens) {
    // your method
}

Called by

if (queryString["secure"].EqualsAny("true", "1")) {
    parameters.Protocol = Protocol.https;
}

Upvotes: 2

user111013
user111013

Reputation:

There's nothing wrong per-se with what you're doing. However this type of functionality already exists in several different fashions.

Example: 

var candidates = List<SomeObject>(); 
if (candidates.Count(c=> string.Compare(c.PropertyValue, queryString["secure"], StringComparison.InvariantCultureIgnoreCase) == 0) > 0)
{
 parameters.Protocol = Protocol.https;
}

Upvotes: 0

JoshBerke
JoshBerke

Reputation: 67068

Another option would be. This will simplify your call site since if you have a couple of strings your matching against you won't have to create the array or list in code.

 public static bool EqualsAny(this string s,StringComparison comparisonType, param string[] tokens )
{
   return EqualsAny(s,comparisonType,tokens);
}    

 public static bool EqualsAny(this string s,StringComparison comparisonType, IEnumerable<string>tokens )    
{ 
    //Throw nullReference to keep the semantics aligned with calling an instance member
    if (s==null) throw new NullReferenceException();       
    foreach (string token in tokens)        
    {            
         if (s.Equals(token, comparisonType))            
         {                
             return true;            
         }        
   }        
   return false;    

}

Upvotes: 3

user1228
user1228

Reputation: 

public static bool EqualsAny(
    this string s, 
    StringComparison comparisonType, 
    params string[] tokens)
{
    foreach (string token in tokens)
    {
        if (s.Equals(token, comparisonType))
        {
            return true;
        }
    }
    return false;
}

With params, you don't have to force your strings into an array first.

var match = "loool".EqualsAny(StringComparison.Ordinal, "hurf", "Durf");

Linq-ified (JC + me) with a NRE (framework standard):

public static bool EqualsAny(
    this string s, 
    StringComparison comparisonType, 
    params string[] tokens)
{
   if(s == null) throw new NullReferenceException("s");
   return tokens.Any(x=> s.Equals(x, comparisonType));
}

Upvotes: 5

Konrad Rudolph
Konrad Rudolph

Reputation: 545568

Make your tokens parameter more general – i.e. make it an IEnumerable<string>.

Also, an equivalent method already exists that extends IEnumerable<>, e.g. Any:

 public static bool EqualsAny(this string s, IEnumerable<string> tokens, StringComparison comparisonType)
 {
     return tokens.Any(t => s.Equals(t, comparisonType));
 }

Also, Joel is of course right: you might want to check for null values before performing the actions (defensive coding). This isn't more secure but it makes the error easier to localize.

Upvotes: 3

Related Questions