I just read this article here: http://hamletdarcy.blogspot.com/2008/04/10-best-idea-inspections-youre-not.html , and the last bit in particular made me think about my code, in particular advice:
What in the world is a public method that runs on your object that is independent of any fields inside the object? This is definitely the smell of code. The problem is that the “automatic fix” for validation is using a static keyword. Nooo. This is not what you want to do. An open method without any dependence on the state of an object cannot be part of an object that has a clearly articulated charter. It is simply not cohesive and should be placed somewhere else. So: if this method is private, accept the automatic fix, but if this method is public, do not.
This code is essentially an object transformer. It takes an object of type A and converts it to another type.
My hierarchy is this:
Interface ObjectTransformer -> GenericObjectTransformer
and then below that, GenericObjectTransformer extends ObjectTransformerA and ObjectTransformerB
Now some functions are required by both ObjectTransformerA and ObjectTransformerB, but in fact they are not dependent on any instance variables of GenericObjectTransformer, therefore its protected static method is in GenericObjectTransformer.
Is this a violation of the rule above? Obviously, this is a protected, not public, but its method, accessible from outside the class, which has nothing to do with the class itself?
Any thoughts?
source
share