Naming Conventions

Jul 15, 2008 at 12:42 PM
Edited Jul 15, 2008 at 12:44 PM
First, let me say that you're off to a nice start here :).  I have a few suggestions for improving the visibility of your extension methods in Intellisense.  By grouping similar methods together with a common prefix, you make them more easily discoverable.

Examples:
  • UrlEncode/UrlDecode -> EncodeUrl/DecodeUrl
  • HtmlEncode/HtmlDecode -> EncodeHtml/DecodeHtml
  • ToBase64/FromBase64 -> EncodeBase64/DecodeBase64
  • Md5 -> EncodeMD5
  • Sha1 -> EncodeSHA1
Other usability suggestions:
  • Dictionary<string, object> ToHash(this object) -> The name "ToPropertyValueDictionary" more accurately describes the method's functionality
  • bool Is(this string, string) -> I don't much see the point in this; it's essentially the same as the String.Equals instance method, except that it does null checking for you.  Also, the implementation uses String.Compare when it probably ought to use the static method String.Equals--why not just use String.Equals in place of this extension method?
  • string Rerverse(this string) -> should be "Reverse" ;)
  • string Sub(this string, params object[]) -> How about renaming this method to "FormatWith"?
  • void Each<T>(this IEnumerable<T>, Action<T>) -> How about "ForEach" instead?
  • bool TrueForAll<T>(this IEnumerable<T>, Predicate<T>) -> This is redundant to bool Enumerable.All<T>(this IEnumerable<T>, Func<T,bool>).
  • bool Exists<T>(this IEnumerable<T>, Predicate<T>) -> This is redundant to bool Enumerable.Any<T>(this IEnumerable<T>, Func<T,bool>).
  • T Find<T>(this IEnumerable<T>, Predicate<T>) -> This is redundant to T Enumerable.First<T>(this IEnumerable<T>, Func<T,bool>).
  • int Index<T>(this IEnumerable<T>, Predicate<T>) -> How about "FirstIndexOf" instead?
Cheers,
Mike
Coordinator
Jul 15, 2008 at 11:57 PM
Thanks for the feedback, I've implemented all your renames - I completely agree. The only differences are:
  • EncodeMD5 -> EncodeMd5 (casing)
  • EncodingSHA1 -> EncodeSha1 (casing)
  • ToPropertyValueDictionary ->  ToDictionary
The key with string.Is is that it's case insensitive by default. Equals isn't. I'm more than happy to change the internal implementation from string.Compare(a, b, true)  to @string.Equals(b, StringComparison.OrdinalIgnoreCase), but I'm not sure what the difference is (they seem vastly different in resharper though)

I [obviously] didn't know about the extensions in System.Linq - extension methods aren't discoverable!  I know I should take them out, but there's something nice about only having to include CodeBetter.Extension for all the extension i need...
Jul 16, 2008 at 3:58 PM
Edited Jul 16, 2008 at 4:00 PM
Hi Karl,

Glad you liked my suggestions :).  I still think "ToDictionary" is a bit odd.  The name doesn't shed a whole lot of light on what the method actually does.  If I have a Customer object, what would I expect a ToDictionary method to do?  What if the object is a collection type?  It becomes a bit more confusing then.  Lastly, there exist Enumerable.ToDictionary(...) methods that will also show up on IEnumerable obects, which further adds to the confusion.

Thinking about it further, I'm not sure "To" is an appropriate prefix here either.  Perhaps "GetPropertyValues" or "GetPropertyValueDictionary" would be a better and more accurate name?  If you're concerned about brevity, perhaps "GetProperties" or "GetPropertyLookup"?

Aside from the name, I had another thought about this method.  Perhaps, instead of returning a Dictionary<K,V>, you could return a custom IDictionary<K,V> implementation that gets/sets (or only gets) the property values on demand, rather than copying all the values into a dictionary when the method is called.  That way the lookups against the dictionary would reflect the current property values rather than a snapshot.

Lastly, String.Compare is really meant for determining the sort order of strings, whereas String.Equals is meant for determining (in-)equality.  Depending on what culture you're in, it's theoretically possible that String.Compare would not always work the way you'd expect.

Cheers,
Mike
Jul 20, 2008 at 9:55 AM
T Enumerable.First<T>(this IEnumerable<T>, Func<T,bool>) -> sucks big time because it throws an exception if the element is not found. Not sure about bool Enumerable.All<T>(this IEnumerable<T>, Func<T,bool>). or Any.