multiple type code switches

[miluch]

Hi

I am working on refactoring quite big, 4 years old, web-based system written in JAVA.
This is typical 3 layered (spring promoted architecture) with UI based on Struts & JSP, Service layer -spring managers accessed by remote EJB 2.1 and DAO layer using Spring LDAP and Hibernate (spring managed of course).

System is used by multiple kind of users: managers, administrators, employees, team leaders, team leader specialist and a few more. About half of methods of each manager/service in the system has big switch statement:

switch(context.getRole() ){
case manger: dosth();
case team_leader : dosthelse();
case: employee: doEvenSthElse();
...
}

Moreover these kind of switch statements are spread all over the system, i also can find them in jsp or within struts action classes - depending on user type different managers are executed or are executed with different (user type specific) parameters

It all looks terrible and is very hard to maintain.

According to Fowler's Refactoring book the best way is to replace these switch statements with polymorphisms. Following Fowler's advise i would start by employing "Replace Type Code with Subclass" or "Replace Type Code with Strategy" and then use "Replace Conditional with Polymorphism".
This way i would encapsulate all user-type dependent behavior into single subclass or interface implementation.

The problem i see with this concept is that my interface would be very big, it would contain great number of methods (about 100 methods). Of course the implementations would delegate processing to other managers but still classes could be very big and would use nearly all managers that exist in the system.
The other problems i can think of now, is how to integrate this kind of solution into Spring DI: create/lookup userType bean in servlet filter and then pass it along as parameter to managers (problems with serialization since using remote EJB 2.1 beans ) or maybe let each manager lookup userType bean itself?

The other solution that comes to my mind is to partition user-type dependent behavior in different manner. Instead of creating one GOD class that encapsulate every user-type dependent behavior, create separate manager implementation for each user type. Using Spring AOP with custom TargetSource(dispatching based on thread local set in servletFilter) Spring would route every request to specific (user-type dependent) manager implementation.Adding new user type would require implementing a multiple interfaces.

Does anyone have any other ideas how to cope with such a problem?

[constv]

I think the last approach you mention is on track. It's difficult to advise with certainty without actually seeing your application. However, based on what you are describing, each user type requires its own way of implementing the same number of operations. If so, it seems it would make sense to, indeed, introduce a generic "manager" interface that would define the operations, and for each type of user (I assume there are only a few, not dozens or hundreds!) provide implementations. You probably would also have some abstract manager class that would implement the common interface and provide the functionality common to all or most user types. The actual type-specific implementations may extend that abstract manager. I don't see the need for aspects here - at least for anything that would implement the manager instances lookups. You could simply maintain a Spring-managed mapper bean into which you would inject the mappings between the user types (those could be actual classes, or, probably just enums, or something like that) and the references to your Spring-managed user-type-specific manager implementation beans.

Then, for each user, whenever necessary, you could do something like this:

@Autowired(Required=true)
private YourManagerMapper managerMapper;

...

// in some method:

   Manager m = managerMapper.getManager(user.getType());
   m.doSomething(...);

HTH, and good luck!

[miluch]

Hi

Thanks constv for an answer. I am not sure if i understand correctly your answer since you are suggesting to follow the second idea (create separate managers for each user type for each existing manager) but you also wrote

introduce a generic "manager" interface that would define the operations, and for each type of user (I assume there are only a few, not dozens or hundreds!) provide implementations

Did you mean to introduce a generic "manager" interface that would define all the operations that are currently spread over different managers (like CustomerManager, TransactionSearchManager, UserManager, CurrencyManager, ... ) or to provide generic managers, each one for each of existing concer/manager?

[constv]

My assumption was that for each user type you have an operation that could be characterized the same way but implemented differently. In other words, your base interface would contain a reasonable number of operations, like "doSomething()", but each implementation of the manager will provide its own version of it tailored for the particular user type. I do understand that in your legacy application, all those methods have different names - because they just had to be named differently, there's no polymorphism at all in the current design. But are those operations all of the same nature for different user types? Or, did I get it wrong, and those are completely different operations? Like "if the user is type A, go mop the floor; if user type B I will make myself a sandwich"... Like, completely unrelated things... If that's the case, then a single neat interface solution might not fit in well at all.

In other words, is there polymorphism (at least potentially) in you user/manager model?

[miluch]

Hi

I am not a native speaker so maybe i am not able to express myself clearly.

My system consists of 20 managers (UserManager, CustomerManager, CurrencyManager , ...). For each of the manager you have a single interface and a single implementation class.
Each manager interface has about 5-7 methods. If you look on the body of each of these methods implementations you could find a lot of switch statements: different code is executed based on user type.
So my first idea was to create new interface: UserTypeOperationManager.
This interface would contain all the methods from all the managers that have a switch statement within their body. It would be like a horizontal cut:
look at each method implementation of each of existing managers, if you find user-type switch there then introduce a new method in UserTypeOperationManager interface.
This way I would encapsulate user type dependent behavior in one BIG interface. If i have a plan to introduce new user type , just implement singe interface UserTypeOperationManager.

The 2nd approach is to have a seperate (user type specific) implementation of each of existing managers. Instead of having a single implementation of UserCustomer interface(each of its methods have switch statement) i would have a multiple implementations. Each implementation would be user type specific. This time if i decide to introduce additional user type i would have to provide new implementations for each of manger interfaces.

I hope it is clear now

[constv]

Is your "manager" essentially a business service? Does "Currency Manager" implement currency-related use-cases, and there should be different implementations for currency manipulations for different user types. Am I getting it right?

There is nothing polymorphic in the first approach you have just described. You would simply be aggregating all you methods from all your managers under the umbrella of a single interface, and then delegating. I don't see how this can simplify anything. Definitely, not a good option.

The second option, indeed, implies polymorphism - all types of users are treated exactly the same by each category of managers. So, if my assumption about your "managers" is correct (and I am not so sure) you would have:

CurrencyManager (interface)
Implementations:
CurrencyManagerForEmployees
CurrensyManagerForTeamLeader
CurrencyManagerForJanitor
etc.

The same for other "categories of managers". Everything in your application will only talk to the interface (e.g. CurrencyManager), and the actual implementation will be resolved on the fly for the user at hand. Not sure why you need to attach anything to ThreadLocal (saw that in your original post.) I envision some controller class - that needs to invoke operations on the currency managers - injected with some CurrencyManagerResolver. Each time such an operation is required for the user, the controller does it not directly on the currency manager instance but via the resolver, passing the user object or user type argument.

  @Autowired(Required=true)
  CurrencyManagerResolver cmr;
  ...

  ...
  // time to call a currency operation
  Currency salary = cmr.getManager(user.getType()).calculateSalary(user);

[chelu]

I see the first aproach stronger because you are using polymorphims to choose the correct method and adding new roles to application is a clear job.

On the second you need to do the dynamic binding manually via hashmap o whatever that is error prone.

But if the size of the Role interface is really big there are another options to see.

I recomends you reading this article about roles to help you finding the best role implementation for you application

http://martinfowler.com/apsupp/roles.pdf

HTH

-- José Luis Martin.

[miluch]

I am glad we are starting to speak common language:

Is your "manager" essentially a business service? Does "Currency Manager" implement currency-related use-cases, and there should be different implementations for currency manipulations for different user types. Am I getting it right?

Yes, you are right.

There is nothing polymorphic in the first approach you have just described. You would simply be aggregating all you methods from all your managers under the umbrella of a single interface, and then delegating. I don't see how this can simplify anything. Definitely, not a good option.

You are wrong, it would be polymorphic behavior since you would see UserTypeOperationManager interface and its implementations:

UserTypeOperationManagerForEmployees
UserTypeOperationManagerTeamLeader
UserTypeOperationManagerForJanitor
...

These implementations would contain system-wide user specific behavior in contrast with manager scoped as proposed in solution 2.
Take a look at example

CurrencyManager(){
doSth(){
switch(userType)
 manager
    doSthForManager();
 employee:
  doSthForEmployee();
 teamleader:
 doSthForTeamLeader
}
}

UserManager(){
doFoo(){
switch(userType)
 manager
    doFooForManager();
 employee:
  doFooForEmployee();
 teamleader:
 doFooForTeamLeader
}
}

so after refactoring my UserTypeOperationManager inerface will have 2 methods:
doSth
doFoo

These methods would be implemented by user type specific implementations.

UserTypeOperationManager implementations will be used (injected) by UserManager and CurrencyManager. That's how i would use polymorphisms to get rid off switch statements.

The second option, indeed, implies polymorphism - all types of users are treated exactly the same by each category of managers. So, if my assumption about your "managers" is correct (and I am not so sure) you would have:

CurrencyManager (interface)
Implementations:
CurrencyManagerForEmployees
CurrensyManagerForTeamLeader
CurrencyManagerForJanitor
etc.

The same for other "categories of managers". Everything in your application will only talk to the interface (e.g. CurrencyManager), and the actual implementation will be resolved on the fly for the user at hand. Not sure why you need to attach anything to ThreadLocal (saw that in your original post.) I envision some controller class - that needs to invoke operations on the currency managers - injected with some CurrencyManagerResolver. Each time such an operation is required for the user, the controller does it not directly on the currency manager instance but via the resolver, passing the user object or user type argument.

  @Autowired(Required=true)
  CurrencyManagerResolver cmr;
  ...

  ...
  // time to call a currency operation
  Currency salary = cmr.getManager(user.getType()).calculateSalary(user);

The second option is very similar to the first one. But in this case instead of encapsulating user type dependent code into 1 interface you are providing user type specific implementations of each of managers.

The thing about thread local and using custom targetSource is a spring mechanism you could employ to get rid off Resolvers/Mappers you proposed. "userType specific managers"/"usertype specific implementations of UserTypeOperationManager" would be properly dependency injected by Spring. If you are not familiar with this mechanism you could think of it as being similar to what aop scoped proxy provides.

[miluch]

I see the first aproach stronger because you are using polymorphims to choose the correct method and adding new roles to application is a clear job.

That's a point i raised in initial post: adding a new type of user requires implementitng just 1 interface. Although this interface would be very big.

On the second you need to do the dynamic binding manually via hashmap o whatever that is error prone.

Even in solution 1 i would like to have user type specific implementations of UserTypeOperationManager interface injected. I can do it using mapper/resolver or do it in the same way as proposed in solution 2.
So injection mechanism (on the fly based on logged in user type)would look identical in both cases

But if the size of the Role interface is really big there are another options to see.

I recomends you reading this article about roles to help you finding the best role implementation for you application

http://martinfowler.com/apsupp/roles.pdf

I would definitely take a look at document. Thanks

[constv]

So, I misread your first approach suggestion, which is basically the same as what I suggested as approach two, with further categorization by manager type. It was difficult to get a clear picture of what your actual situation was based on the original posts. As for the resolver, it is exacltly the mappings you are talking about - injected via Spring configuration. I have not done that using ThreadLocal. Also, I am in no position to assume that you are planning to use it only in the web tier, or also inject your "managers" into any middle-tier beans, or if they may be distributed. That is why I was cautions about ThreadLocal and relying on the servlet context.

Well, good luck with your refactorings! Hope it all works out well.