56

This question is subjective but I was just curious how most programmers approach this. The sample below is in pseudo-C# but this should apply to Java, C++, and other OOP languages as well.

Anyway, when writing helper methods in my classes, I tend to declare them as static and just pass the fields if the helper method needs them. For example, given the code below, I prefer to use Method Call #2.

class Foo
{
  Bar _bar;

  public void DoSomethingWithBar()
  {
    // Method Call #1.
    DoSomethingWithBarImpl();

    // Method Call #2.
    DoSomethingWithBarImpl(_bar);
  }

  private void DoSomethingWithBarImpl()
  {
    _bar.DoSomething();
  }

  private static void DoSomethingWithBarImpl(Bar bar)
  {
    bar.DoSomething();
  }
}

My reason for doing this is that it makes it clear (to my eyes at least) that the helper method has a possible side-effect on other objects - even without reading its implementation. I find that I can quickly grok methods that use this practice and thus help me in debugging things.

Which do you prefer to do in your own code and what are your reasons for doing so?

5
  • 3
    I would remove C++ from this question: in C++, obviously, you would make it a free method, because there's not point for it being arbitrarily stashed into an object: it disables ADL. Oct 3, 2011 at 18:11
  • 1
    @MatthieuM.: Free method or not, it doesn't matter. The intent of my question was to ask if there are any advantages in declaring helper methods as instance ones. So in C++ the choice could be instance method vs free method/function. You've got a good point about ADL. So are you saying that you'd prefer using free methods? Thanks!
    – Ilian
    Oct 3, 2011 at 23:56
  • 2
    for C++, free methods are recommended. They increase encapsulation (indirectly, as being only able to access the public interface) and still play nice because of ADL (especially in templates). However I do not know if this is applicable to Java/C#, C++ differs greatly from Java/C# so I expect the answers to differ (and thus I don't think asking for the 3 languages together make sense). Oct 4, 2011 at 6:17
  • duplicate: stackoverflow.com/questions/3604137/… :)
    – nawfal
    May 12, 2013 at 19:18
  • 1
    Without trying to aggravate anyone, I've never heard a good reason not to use static methods. I use them wherever I can since they make it more obvious what the method does. Aug 13, 2015 at 20:06

3 Answers 3

24

This really depends. If the values your helpers operate on are primitives, then static methods are a good choice, as Péter pointed out.

If they are complex, then SOLID applies, more specifically the S, the I and the D.

Example:

class CookieJar {
      function takeCookies(count:Int):Array<Cookie> { ... }
      function countCookies():Int { ... }
      function ressuplyCookies(cookies:Array<Cookie>
      ... // lot of stuff we don't care about now
}

class CookieFan {
      function getHunger():Float;
      function eatCookies(cookies:Array<Cookie>):Smile { ... }
}

class OurHouse {
      var jake:CookieFan;
      var jane:CookieFan;
      var cookies:CookieJar;
      function makeEveryBodyAsHappyAsPossible():Void {
           //perform a lot of operations on jake, jane and the cookies
      }
      public function cookieTime():Void {
           makeEveryBodyAsHappyAsPossible();
      }
}

This would be about your problem. You can make makeEveryBodyAsHappyAsPossible a static method, that will take in the necessary parameters. Another option is:

interface CookieDistributor {
    function distributeCookies(to:Array<CookieFan>):Array<Smile>;
}
class HappynessMaximizingDistributor implements CookieDistributor {
    var jar:CookieJar;
    function distributeCookies(to:Array<CookieFan>):Array<Smile> {
         //put the logic of makeEveryBodyAsHappyAsPossible here
    }
}
//and make a change here
class OurHouse {
      var jake:CookieFan;
      var jane:CookieFan;
      var cookies:CookieDistributor;

      public function cookieTime():Void {
           cookies.distributeCookies([jake, jane]);
      }
}

Now OurHouse need not know about the intricacies of cookie distribution rules. It must only now an object, which implements a rule. The implementation is abstracted away into an object, who's sole responsibility is to apply the rule. This object can be tested in isolation. OurHouse can be tested with using a mere mock of the CookieDistributor. And you can easily decide to change cookie distribution rules.

However, take care that you don't overdo it. For example having a complex system of 30 classes act as the implementation of CookieDistributor, where each class merely fulfills a tiny task, doesn't really make sense. My interpretation of the SRP is that it doesn't only dictate that each class may only have one responsibility, but also that a single responsibility should be carried out by a single class.

In the case of primitives or objects you use like primitives (for example objects representing points in space, matrices or something), static helper classes make a lot of sense. If you have the choice, and it really makes sense, then you might actually consider adding a method to the class representing the data, e.g. it's sensible for a Point to have an add method. Again, don't overdo it.

So depending on your problem, there are different ways to go about it.

21

It is a well known idiom to declare methods of utility classes static, so such classes need never be instantiated. Following this idiom makes your code easier to understand, which is a good thing.

There is a serious limitation to this approach though: such methods / classes can't be easily mocked (although AFAIK at least for C# there are mocking frameworks which can achieve even this, but they aren't commonplace and at least some of them are commercial). So if a helper method has any external dependency (e.g. a DB) which makes it - thus its callers - hard to unit test, it is better to declare it non-static. This allows dependency injection, thus making the method's callers easier to unit test.

Update

Clarification: the above talks about utility classes, which contain only low level helper methods and (typically) no state. Helper methods inside stateful non-utility classes are a different issue; apologies for misinterpreting the OP.

In this case, I sense a code smell: a method of class A which operates primarily on an instance of class B may actually have a better place in class B. But if I decide to keep it where it is, I prefer option 1, as it is simpler and easier to read.

3
  • Can't you just pass the dependency to the static helper method?
    – Ilian
    Oct 2, 2011 at 13:57
  • 1
    @JackOfAllTrades, if the sole purpose of the method is to deal with an external resource, there is hardly any point injecting that dependency. Otherwise, it may be a solution (although in the latter case, the method probably breaks the Single Responsibility Principle, thus is a candidate for refactoring). Oct 2, 2011 at 14:21
  • 1
    statics are easily 'mocked' - Microsoft Moles or Fakes (depending, VS2010 or VS2012+) allows you to replace a static method with your own implementation in your tests. Makes the old mocking frameworks obsolete. (it also does traditional mocks too, and can mock concrete classes as well). Its free from MS through extension manager.
    – gbjbaanb
    Sep 11, 2013 at 18:32
4

Which do you prefer to do in your own code

I prefer #1 (this->DoSomethingWithBarImpl();), unless of course the helper method does not need access to the instance's data/implementation static t_image OpenDefaultImage().

and what are your reasons for doing so?

public interface and private implementation and members are separate. programs would be far more difficult to read if i opted for #2. with #1, i always have the members and instance available. compared to many OO based implementations, i tend to use a lot of encapsualtion, and very few members per class. as far as side effects - i am ok with that, there is often state validation which extends the dependencies (e.g. arguments one would need to pass).

#1 is much simpler to read, maintain, and has good performance traits. of course, there will be cases where you can share an implementation:

static void ValidateImage(const t_image& image);
void validateImages() const {
    ValidateImage(this->innerImage());
    ValidateImage(this->outerImage());
}

If #2 were a good common solution (e.g. the default) in a codebase, I would be concerned about the structure of the design: are the classes doing too much? is there not enough encapsulation or not enough reuse? is the abstraction level high enough?

lastly, #2 seems redundant if you are using OO languages where mutation is supported (e.g. a const method).

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.