Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

My team has been developing a web business application for more than a year. It started quite small, but now it is growing bigger and bigger. I think it's time for refactoring.

Looking at the code, I see the below situation:

IUserService

  • addUser
  • deleteUser
  • updateUser
  • addMember
  • deleteMember
  • updateMember
  • registerMember
  • deactivateMember
    ....
  • getExpiringMember
  • nominateMember

Generally saying, there are about 50 functions for more than 2500 lines of business logic. I once read "Clean code", it states that there should be no more than 200 lines for a normal class. Our code violates this requirement seriously, and I'm beginning to feel the hardship of code management.

The simple solution might be spliting this service into many other services, but it is not feasible/reasonable:

  1. This service class serves as an API point for mobile client, so that every change on Interface (even renaming) requires change for the client as well. I don't want to spend the precious time of our team if not for good reason.

  2. I already split a part of this service into another classes, but that class (RegistrationService) also grow quite fast and reach 500 lines of code recently.

  3. More and more business logic request recently, and these services are going to grow even more.

I'm afraid that at some point we will lose the tracks if a team member who manage a specified service got retired.

Is there any way I can make this service more manageable? Any Design Pattern, Refactoring technique that I can apply? I'm still not experienced enough in this field... any suggestions or directions are welcome.

share|improve this question

2 Answers 2

up vote 3 down vote accepted

I'm not a Spring expert but from general design/architecture experience this looks like way too many methods for a single API / Service. It's something of a design smell to see this many methods in one interface.

You should try an break it down into logically distinct areas of functionality, each with a separate interface. Then the code to implement each distinct area should become manageable.

If you have client code that is hard to change, then it's fine to keep the old version of the service - just turn it into a lightweight facade that calls into the new refactored versions. Your clients won't have to change anything if they don't want to (though you should make clear that they will have to move to the new interfaces if they want access to any future functionality).

From the just the methods listed in the question above you could have at least four separate domains:

  • User management
  • Member management
  • Registrations (registerMember etc.)
  • Nominations (nominateMember etc.)
  • etc.
share|improve this answer
    
thanks, I can't think that I missed the facade concept. It looks like a good way to start. –  Hoàng Long Jul 21 '12 at 7:18

It seems you can't split your service interface into different classes / services, to maintain backwards compatibility.

What you should do though, is split the functionality into classes according to SRP, and then (best-case) have interfaces to those classes as dependencies in the constructor of your service class or (worse-case) instatiate them all in the constructor.

Each method in the service class then becomes a single line of wrapping goodness, and the rest of your application can be sanely thought out without changing the public API.

Example:

public class Service : IUserService
{
    private IUserRepository userRepo;
    private IMemberStuff memStuff;

    public Service(IUserRepository userRepo, IMemberStuff memStuff)
    {
       this.userRepo = userRepo;
       this.memStuff = memStuff;
    }

    public void addUser(...)
    {
        userRepo.addUser(...);
    }

    public void deleteUser(...)
    {
    }

    public void updateUser(...)
    {
    }

    public void addMember(...)
    {
    }

    public void deleteMember(...)
    {
    }

    public void updateMember(...)
    {
    }

    public void registerMember(...)
    {
    }

    public void deactivateMember(...)
    {
    }

    public Member getExpiringMember(...)
    {
    }

    public void nominateMember(...)
    {
    }
}
share|improve this answer
    
hi Max, sorry for my late reply. I got quite busy recently. By the way, what's the problem with initializing in constructors? I get used to @Autowire other services into my classes, but also see many others who like to initialize their helper classes right before they use them –  Hoàng Long Jul 24 '12 at 9:31

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.