Public static methods - a bad sign?

Question

I've just read this article here: http://hamletdarcy.blogspot.com/2008/04/10-best-idea-inspections-youre-not.html, and the last bit in particular got me thinking about my code, specifically the advice:

What in the world is a public method doing on your object that has no dependency on any fields within the object? This is certainly a code smell. The problem is that the "auto-fix" for the inspection is to apply the static keyword. Nooooo. That's not what you want to do. A public method without any dependency on object state can't possibly be part of an object that has one clearly stated charter. It's just not cohesive and should be placed somewhere else. So: if the method is private, accept the auto-fix, but if the method is public then don't.

The code in question is essentially an object transformer. It takes an object of type A and converts it to a different type.

My hierarchy is like this:

Interface ObjectTransformer -> GenericObjectTransformer

and then below this, GenericObjectTransformer is extended by ObjectTransformerA and ObjectTransformerB

Now, some functionality is required by both ObjectTransformerA and ObjectTransformerB, but doesnt actually depend on any instance variables of GenericObjectTransformer, so its a protected static method in GenericObjectTransformer.

Is this a violation of the rule above? Obviously this is protected rather than public, but its still a method accessible from outside of the class that has nothing to do with the class itself?

Any thoughts?

Answer 1

I disagree with the excerpt you pulled.

A public method without any dependency on object state can't possibly be part of an object that has one clearly stated charter. It's just not cohesive and should be placed somewhere else. So: if the method is private, accept the auto-fix, but if the method is public then don't.

Just because a method is static and has no relation to state, doesn't mean it falls under the "low cohesion" category. Cohesion/Functionality isn't based on state.

When you are trying to determine Cohesiveness think about the role of the class as a whole, not just the instance variables. If the logic you are looking at is related to the generic concept (GenericObjectTransformer) then leave it there.

If it is a routine to calculate the orbit of the moon, or the depth of the ocean move it to a utility class (another smelly area of our field).

Answer 2

It feels slightly unclean, but is seem preferable to the alternatives I can think of.

I think that the original

A public method without any dependency on object state can't possibly be part of an object that has one clearly stated charter.

You reference is too black and white, and your situation is even greyer.

By having your protected method you are nicely documenting that its intended for use by derived classes. If you don't put it in the base class, then presumbly it's got to go in some ObjectTransformUtility class. Is that win? More artefacts, more places to look.

One thought: if your ObjectTransormer class undergoes significant change then how likely are you to need to change these utility methods. After all if their business is to work agains the object's interface then in fact their cohesion is quite high.