Usage of static methods

[sap]

Usage of static methods

[HrFlorianHoffmann]

One of the main reasons we advice against static methods is that it is hard to test them.

In the following example, it is hard to unit-test the method_under_test because the static dependency to unmockable_method cannot be mocked:

METHOD method_under_test.
  some_dependency=>unmockable_method( ).
ENDMETHOD.

With objects, this becomes easy, as the dependency can be replaced with a test double:

METHOD method_under_test.
  some_dependency->mockable_method( ).
ENDMETHOD.

How would you make this testable without instance methods?

[sbcgua]

I think we are talking about different aspects.

Let's consider an example.

class lcl_my_class
  data mt_data type table.
  methods run.
  methods select_data.
  methods post_process_data.
endclass.

method select_data.
  select * from xxx where yyy into mt_data.
endmethod.

method post_process_data.
  loop at mt_data assigning <i>.
    <i>-attr = ...
  endloop.
endmethod.

method run.
  select_data( ).
  post_process_data( ).
endmethod.

The code is OK. However it can be improved. The run method serves kind of controller - the main flow. And although the logic flow is clear, the data flow is obscured by usage of members inside secondary methods. In terms of readability and comprehension I would declare select and post_process as static and thus get this.

method run.
  mt_data = select_data( ).
  post_process_data( 
    changing 
      ct_data = mt_data ).
endmethod.

Now the data flow also clearer and post_process_data becomes less error prone as it does not relay on object state (postprocessing is not the right example for this issue I confess). Object state is a minified global variables problem to some extent. Pure functions just do now have this problem conceptually. Also post_process becomes more testable in terms of isolation (if you want to test it at all ... see below).

Now let's focus select. It is accessing DB so good idea to mock it. In this case you create an interface, a data accessor class implementing this interface, well ... if I understand correctly this is the case you're referring to. And I 100% agree, hands up. The select should become instance method (of data accessor) in this case.
Note that the above code is very refactoring-friendly for select, just one clean change to do:

method run( data_accessor ). " PSEUDO CODE
  mt_data = data_accessor->select_data( ).
  post_process_data( 
    changing 
      ct_data = mt_data ).
endmethod.

The above is more about code composition and being more bug prone and readable. In terms of testability:

• I agree that public methods should better be mockable
• static public are probably utilities and should go to a separate class. E.g. let's say we also have some method convert_account_to_range which converts single or table of e.g. gl accounts into range table. This really should be in-out pure function - a static method. But if it remains public then it does not belong to the class.
• finally the post_process_data would probably be private. And then maybe it should not even be tested ("test public interface ..." rule) so it end up with benefits of having it as a pure function I mentioned.

To summarize: it is not about "avoiding static methods" it is about "properly using them". I personally would forumulate the rule as "unless you're planning to mock the object, try to use pure functions as far as you can while it stay convenient and clean (mainly about number of passed parameters and performance)"

[HrFlorianHoffmann]

What you are describing is the conflict between stateful and stateless programming. As described in Don't mix stateful and stateless in the same class, both are sound and have their applications.

Also, as described in the examples above, both benefit from instances, objects, and interfaces, and should not be implemented with static methods alone.

Do we need to reformulate Prefer objects to static methods to make clear that the "object" here really only means "instances of classes", and not "object-oriented stateful design"?

[HrFlorianHoffmann]

@sbcgua, I am unclear what to do with this issue. Does it need more clarification? Do we need to make the text clearer, as described in my last post above?

[sbcgua]

I guess let's close for now. "Prefer objects to static methods" section does capture my idea about utilities. I think it can and should be used wider but at the end it is a matter of preference. :)

(And sorry for long silence :)