Tutorial :My first extension method, could it be written better?



Question:

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!


Solution:1

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));  }  


Solution:2

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.


Solution:3

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;        }  


Solution:4

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.


Solution:5

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;  }  


Solution:6

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;  }  

Note:If u also have question or solution just comment us below or mail us on toontricks1994@gmail.com
Previous
Next Post »