Search code examples
javaspringclass-design

How to work with spring when refactoring bulky classes


This is a bit of a generic question about java class design and how that interacts with spring but I will try to use an example to make it a little easier to talk about. (Note that this code is mostly psedo code, lots of fluff has been excluded).

Imagine we encounter a class that is large and poorly factored. It is doing many things which makes it harder to reuse, test or understand. In my example we have a "BulkyThingDoer" class that both does a thing and also tracks statistics on how many times its done a thing and how long it takes (for publishing to JMX). Not terrible but as the number of statistics goes up the class becomes dominated by bookkeeping and the actual important thing its doing gets lost in the noise. If we have a competing DoerInterface implementation we would need to reproduce the same stats tools etc.

@Component
@ManagedResource
public class BulkyThingDoer implements DoerInterface, DoerStatistics{
  private int thingsDone = 0;
  private long timeSpentDoingThings = 0;

  public void doThing(){
    long start = System.currentTimeMillis();
    // do the important thing
    thingsDone ++;
    timeSpentDoingThings += System.currentTimeMillis() - start;
  }
  public int getThingsDone(){ return thingsDone;}
  public long getTimeSpentDoingThings (){ return timeSpentDoingThings;}
}

Lets now imagine that we refactor this class, split out all of the statistic bookkeeping into a helper object and then have the actual doer just report (at the highest level possible) what occured. Now both the ThingDoer and the DoerStatistics objects are simple, testable, Single-responsibility objects.

@Component
public class ThingDoer implements DoerInterface {
  private final DoerStatistics stats;

  public ThingDoer(DoerStatistics stats){
    this.stats = stats;
  }

  public void doThing(){
    long start = System.currentTimeMillis() - start;
    // do the important thing
    stats.recordThatWeDidThing(System.currentTimeMillis() - start);
  }
}

@ManagedResource
public class DoerStatisticHolder implements DoerStatistics{
  private int thingsDone = 0;
  private long timeSpentDoingThings = 0;

  public void recordThatWeDidThing(long duration){
    thingsDone ++;
    timeSpentDoingThings += duration;
  }
  public int getThingsDone(){ return thingsDone;}
  public long getTimeSpentDoingThings (){ return timeSpentDoingThings;}
}

To me it is clear that this would be a good refactoring to embark on, I think this is extending the DI principle all the way down to details the implementation.

This was all a leadup to this issue: Once I have done this refactoring my class becomes much harder to use in a spring context file. Where before I could just use a single definition and the same object would be created as a bean I can give (or autowire) to other objects I now have to define multiple parts in my context and wire them all together manually. This exposes much more implementation detail to the user than is necessary.

One alternative is to then effectively rebuild the "BulkyThingDoer" helper class to construct my object with right pieces:

public class DelegatedBulkyThingDoer extends DoerInterface{
  ThingDoer doer;

  public DelegatedBulkyThingDoer(){
    DoerStatistics stats = new DoerStatisticHolder();
    doer= new ThingDoer(stats );
  }

  public void doThing(){
    doer.doThing();
  }
}

That solves some of the usability problem, but now I don't get any of the spring magic on either the DoerStatisticHolder or the ThingDoer. In this example that would mean the JMX was never registered. If there was an easy way to inform spring of these sub-objects being created that would probably go along way to solving my issue.

I could of course delegate all of the calls we had in the original Bulky implementation and remove all of the annotations in the two implementation classes but that sounds incredibly inelegant to me. I would effectively have rebuilt the BulyClass again which is especially annoying as it was only necessary for cross-cutting concerns like JMX or logging.

I would argue its that painfulness that lead to the original implementation being as large and complex as it was. Spring seems to induce the developer to stop applying rigorous OOP principles at the implementation level because of difficulties like this. In the springified code-bases I have delved into this pattern appears over and over, the classes end up getting implemented in a single class to support the "high level" spring-context use (single bean def) and not furthur broken down because we lose the spring magic when that happens.

I am relativley new to spring and java (been in c++, scala and ruby for the last few years) so there may something I am missing. How do java/spring developers handle this "impedance mismatch" between spring usability and well factored implementations? Do they bite the bullet and create the DelegatingBulkyClass, force the user to construct my refactored impl classes or just stick with the BulkyClass pattern because it is easiest?


Solution

  • Like the BulkyThingDoer you can annotate the DoerStatisticHolder with @Component so it will be managed by spring and then you can autowire it with @Autowire into the ThingDoer. One drawback is that you also need a non-argument constructor.

    If you always need a new instance of an object you can annotate it additional with @Scope("prototype").

    Using the annotation based configuration aproach for autowiring classes with only one real implementation saves much xml configuration overhead.