tags:

views:

116

answers:

2

I've got a class with some functions which realistically are just 'helper' methods that client code could do itself with other public accessor properties/methods, and I'm undecided as to whether I should define these as properties with a getter, instance methods, or static methods that take the instance as a parameter. In addition, I've also got an interface extracted from the class that is used pretty much everywhere except on construction, to allow my code to use any class implemented against the interface.

The question is, which is best from a design point of view? For example, as a means of getting the initial from this class:

class Person : IPerson {
  private string name;

  public string Name { get { return this.name; } }

  // Property with getter
  public string Initial { get { return this.name.Substring(0,1); } }

  // Instance method
  public string GetInitial { return this.name.Substring(0,1); }

  // Static method
  public static string GetInitial(IPerson person) {
    return person.Name.Substring(0,1);
  }
}

The property lends itself to shorter, more readable client code, but would require anyone implementing against IPerson to write their own implementation, as would the instance method.

The static method would mean implementing classes wouldn't need to write their own, and my code can guarantee how the initial is determined based on the name, but it means it can't be on the interface, and client code's a little more verbose.

Does it just come down to whether not it's a good idea to allow implementing classes to specify how helper methods are calculated?

EDIT: Minor aside, why won't SO let me add the best-practices tag?

+5  A: 

How about an extension method?

namespace IPersonExtensions
{
    public static class IPersonExtensionClass
    {
        public static string Initial(this IPerson @this)
        {
            return @this.name.Substring(0, 1);
        }
    }
}

Use it like this:

string initial = person.Initial();

This way, you can share implementation without having to inherit or rewrite code. Using the separate namespace makes it possible for users to choose whether they want to use this code or not.

John Fisher
Didn't think of that one.. although I'm guessing that method should probably return a string :)
Flynn1179
It seems that Flynn is creating this class from scratch. If that is the case, an extension method should not be used as you have access to the source code. Extension methods should be used when you cannot modify or extend the base class. MSDN General GuideLines http://msdn.microsoft.com/en-us/library/bb383977.aspx "In general, we recommend that you implement extension methods sparingly and only when you have to. Whenever possible, client code that must extend an existing type should do so by creating a new type derived from the existing type."
sgriffinusa
@Flynn: I must have fixed it while you were typing your comment. :)
John Fisher
@sgriffinusa: The ability to provide shared information for an interface is precisely what Microsoft does with Linq. How else would Flynn provide implementation for an interface, anyway? (If IPerson where an abstract base class, that would be different.)
John Fisher
@sgriffinusa: Flynn is providing a default implementation for an interface method here using extension methods, which is a neat solution IMHO.
Alan
To be honest though, given that extension methods are essentially just syntactic sugar for `IPersonExtensionClass.Initial(IPerson person)`, is there really any difference to using a static helper, other than marginally tidier client code?
Flynn1179
@Flynn1179: There isn't, but the extension method is quite a bit tidier. I think it's perfectly valid in this case. The extension method isn't extending the class, it's merely calling methods on its public interface.
Peter Ruderman
@Flynn: Yes, the extension methods are just syntactic sugar, but the important part is that it fits better with how most developers think now. Instead of *operation -> object* (which the helper class requires), the user can think the way intellisense works: *object -> operation*. It definitely helps with discoverability.
John Fisher
This actually makes my code a bit tidier too, as the helper methods are in a separate code file. Also, the methods are on the interface, rather than on the class, which makes more sense.
Flynn1179
A: 

I go back and forth, but I've starting leaning toward making them non static.

Case for static: Forces you to declare that the function doesn't use member variables and leave it stateless. If you accidentally let state creep in by needing member variables, the compiler will warn you and you have to make a concrete decision.

Case for a member function: Static functions are hard to inherit from, so changing the behavior is difficult. Makes testing more difficult as well since it's harder to substitute that function to test something else. Say you had something that used Initial and you wanted to return something specific for a test...

As an aside (and an extreme example), some of my extensions use a IOC container to locate a concrete class and call a member on it.

Alan Jackson