I'm trying to refactor some legacy code that's quite a mess. We're talking a class with 7000 lines and a handfull of methods that are violating the Single Responsibilty Principle so much it hurts to look at. I've managed to break it up a little, but I find I'm mainly just shuffling the code around into separate classes - the complexity is still there. My methods are getting huge lists of parameters because everything is so tangled.
My aim is to add unit testing, but also make the code more maintainable as a result.
I'm working through the code from the "lowest points" (those that touch a database) and trying to refactor them out into classes that do specific things. The database access is nice and easy, I can pull that in to a "DataAccess" class.
However, I'm having trouble with some of the other methods. There's some areas that are doing a single general thing, say - loading some data. My problem is these methods all call each other in one long chain.
For example, here's some psudocode demonstrating the what I've got:
class SensorDataLoader
{
//injected dependency for data access
private readonly DataAccess dataAccess;
// loads all sensors
public void LoadAllSensorData(int dataSetId, LoadParametersObject loadParameters, DataStoreObject dataStore, Configuration configuration, Report report)
{
foreach (var sensorDataStore in dataStore.sensorsToLoad)
{
if (!sensorDataStore.HasDataToLoad) continue; // these parameters have lots of complexity. I've cut out a lot of stuff like this that access the configuration objects, or the data store objects.
LoadSensorData(dataSetId, sensorDataStore, configuration.SensorConfiguration);
}
}
// load a single sensor (i.e. all it's 'tracks')
public void LoadSensorData(int dataSetId, SensorDataStoreObject sensorDataStore, SensorConfiguration sensorConfiguration, Report report)
{
foreach (var trackDataStore in sensorDataStore.tracks)
{
// here I've tried to create some dependencies that I can mock to test later methods
var trackNumberAllocator = new TrackNumberAllocator(trackDataStore.trackId, sensorConfiguration);
var postProcessor = new TrackPostProcessor(sensorConfiguration, trackDataStore.trackId, trackNumberAllocator);
if (sensorConfiguration.someCondition)
{
LoadTrackData(dataSetId, trackDataStore, postProcessor, sensorConfiguration, report);
}
else
{
report.Report(...);
}
}
}
// loads a single 'track' in the sensor
public void LoadTrackData(int dataSetId, TrackDataStoreObject trackDataStore, PostProcessor postProcessor, SensorConfiguration sensorConfiguration, Report report)
{
//get the data
var data = this.dataAccess.GetData(dataSetId, sensorConfiguration);
postProcessor.PostProcess(data);
postProcessor.CalculateOffsets(data);
report.Report(...);
}
}
I've got three methods, that area all loading a sensor (if I split them into separate classes I'm basically going to have to have a separate class for every single method in this code!)
I want to test the methods in isolation, so I can test each one's functionality, but they're chained together.
After struggling to untangle this code, I've also realized that the structure of these classes is also really coupled with the strucutre of both the configuration objects, and 'data store' objects.
All these methods take a really complex object, and mutate it in some way. It's really tricky to untangle them because of it.
Are there any design patterns that could help me with this? I think I'm being a little too naive in simply trying to partition them off into separate classes and something more drastic is needed to separate concerns but I'm struggling to find a design pattern that fits.