Why should 'getter' methods in 'Manager' classes not be static?

Question

Conventional programming wisdom seems to discourage the use of static methods in most cases. Often, I have these 'managers' e.g. UserManager, AppointmentManager e.t.c. Invariably, one of the methods in the manager is XXX getXXX(long xxxId) e.g. User getUser(long userId). I really don't see why this cannot be a static method. It seems very much like a factory method (a la GoF factory pattern). It's hard to pass up the convenience of:

User user = UserManager.getUser(id);

and use

UserManager userManager = new UserManager(); User user = userManager.getUser(userId);

instead.

P.S. I believe in testing; I'm just not a 'mock-testing' fan, so I need reasons besides mocking.

I'm aware of that. I couldn't find a better name for them though. — Emmanuel, Jun 28 '16 at 00:12
.. and use dependency injection instead. `new UserManager()` each time you use the class just for the sake of avoiding `static` methods is IMO the worst approach of them all. — zapl, Jun 28 '16 at 00:42

score 6 · Accepted Answer · answered Jun 28 '16 at 00:09

6

The main reason to avoid static methods in object factories is the ability to keep state. Although static methods can keep their state in static fields, the approach makes it hard to save and reset the state of your factory.

In addition, it becomes impossible to program to interface of your factory, because static methods cannot be used as interface implementations. This becomes important when you need to switch implementations of your objects transparently to the rest of your application.

Finally, static methods make it harder to test your code, with or without mocking. It will be very hard for your tests to verify that certain methods of your factory are being called in a specific order.

answered Jun 28 '16 at 00:09

Sergey Kalinichenko

714,442
84
1,110
1,523

For state saving, I can hardly imagine where I need that, I hope you can elaborate a little. For programming to the interface, perhaps that's another question 'cause, quoting Joshua Bloch "Design and document for inheritance or else prohibit it". In these cases, I'm neither expecting nor planning for inheritance, so coding to the interface if not something (in these cases) that seems relevant. Finally, while I'm no testing genius, the only times where I have verified my tests by ensuring they were methods were called in a specified order was when I mocked. Outside that, not really. – Emmanuel Jun 28 '16 at 00:42
@Emmanuel Object factory may need to keep track of objects that has been created through it, in a way becoming a temporary owner of the objects that it has created prior to the objects being persisted. You may want to save the state of unsaved objects in a temporary location when your application exits, so that you could restore its state on restart, without having to ask end-users if they would like to discard or keep their unsaved work on exit. – Sergey Kalinichenko Jun 28 '16 at 01:15
@Emmanuel As far as programming to interface goes, it does not require programming for inheritance. It may be done simply to add a degree of insulation between programmers using the factory and programmers supplying a factory implementation. This is useful in situations when multiple teams are working on different parts of the project: you set up an interface upfront, and then the teams separately develop code supplying and consuming the interface. – Sergey Kalinichenko Jun 28 '16 at 01:24

score 1 · Answer 2 · answered Jun 28 '16 at 00:45

I think Uncle Bob does a great job explaining this in his Clean Code book (awesome read btw). But anyway, his point is that you should not be using statics anywhere you want to leverage polymorphism (which I think is precisely what you want for the above case).

In your case, you have a UserManager. By no means a complete application, right? You might have something more complicated that uses a UserManager. Let's say you have your own version of StackOverflow (don't do this of course, stackoverflow is awesome, no need to compete).

Okay, so we have a LoginService that calls UserManager.getUser(). This is an unchangeable dependecy (since we aren't leveraging polymorphism). If UserManager.getUser() requires an underlying SQL database, then guess what you need to run (or test) LoginService.... a SQL database!

public class LoginService {
   public boolean authenticate(String username, String password) {
      User user = UserManager.getUser(username); // hard dependency on implementation
      // other stuff
   }
}

The more prevalent solution is to abstract things that can change behind an interface. That way you can swap out implementations. LoginService has a job that should be tested and really shouldn't depend on a specific database implementation.

public interface UserManager {
   User getUser(String id):
}

public class SQLUserManager implements UserManager {
   @Override
   public User getUser(String id) { // SQL stuff }
}

class LoginService {
   public LoginService(UserManager userManager) {
      this.userManager = userManager;
   }

   public boolean authenticate(String username, String password) {
      User user = userManager.getUser(username);
      // other stuff
   }
}

Now LoginService can 1) be tested independently of what UserManager is used and 2) can be left alone if the user implementation changes.

It's not about mocking but testing your components without needing to setup an entire application stack.

score 0 · Answer 3 · answered Jun 28 '16 at 00:45

Keeping state statically is generally discouraged in favour of using dependency injection. As dasblinkenlight mentioned, static methods cant implement interfaces . Using static state makes it impossible to have multiple instances of the class.
If you ever need two different usermanagers pointing to different datasources you have to do major re-factoring.

Why should 'getter' methods in 'Manager' classes not be static?

3 Answers3