How can I split tightly coupled code in an OOP language?

Question

So I've a class that is some kind of wrapper of a state machine for a multipart upload and it's database writes/reads. Everytime something is uploaded via REST basically the following happens

MyMultipartUpload mpu = multipartUploadFactory.create(...);
mpu.load(request.mpuID); //sets mutex lock
if (mpu.state != MpuState.UPLOAD) {
  throw...
}
mpu.addPart(hugeBinaryBlob); //mpu hold a persistency interface which allows saving to the database
...

Now I've a mpu.process() method that can be called when all parts arrived

MyMultipartUpload mpu = multipartUploadFactory.create(...);
mpu.load(request.mpuID); //sets mutex lock
if (mpu.state != MpuState.UPLOAD_COMPLETED) {
  throw...
}
mpu.process();
...

The mpu.process() method loads all parts one by one and first calls the private method mpu.validate(), does some data transformation, saves the new representation form and then triggers an async job.

// process()
List<Part> parts = persistency.loadPartsForId(mpuID);
for (Part part : parts) {
  var result = validate(part);
  if (result.containsErrors()) {
    setState(MpuState.ERROR);
    return;
  }

  var newForm = transformData(part);
  persistency.savePart(newForm);
}

Now my MyMultipartUpload coding has become super long (both validate and transformData have many LoC) and I'm wondering how I could refactor it into more classes (which would also help with unit tests).

What I thought about so far:

1. Have a ValidationService and TransformationService interface, where I can pass the part's content and do the validation / transformation. The service implementation is passed to the constructor of MyMultipartUpload. Saving in the database etc must still be done by the caller.

2. Have a ProcessingService interface, where I pass a whole MyMultipartUpload object. Here the service would have to do the saving of data and validation/transformation itself. This means I need to create a service with access to the persistency (same database tables MyMultipartUpload writes to via the persistency interface). This also means it must be ensured that the lock on the object is present while processing.

Is either of them a good approach?

Answer 1

"both validate and transformData have many LoC"

Inside those methods, do you have sections of code that you can demarcate in some way, if asked to explain what the code does? E.g., do you have if-blocks + some related code around them embedded in a larger piece of logic? Or sections that you've prefaced with a comment explaining what the code below does? If so, perhaps a good way to start is to extract those into methods with well-defined signatures. (Think about things like what kinds of parameters you can pass, should you access a class field or pass it as a parameter, etc. If unsure, pick an approach, then change it later as you restructure.) If you have such "header comments", use them to decide how to name your new methods; ideally, you want method (and parameter) names that make the comments mostly (or entirely) redundant (when one reads the method call, one should gain the same information as when reading the comment - meaning that you can delete the comment).

Then see if there are groups of methods that are at different levels of abstraction (some do lower level tasks, others call them), or groups of methods that are largely unrelated, or that only use a subset of your class fields, etc. These represent concepts that sort of exist in your codebase, but aren't explicitly expressed in your code. Tweak the code a bit further if necessary, so that you can extract those into separate, more focused classes that you can either create and use internally in the parent class, or inject into a constructor, if you want to allow for more flexibility, or replace these dependencies with a mock for testing purposes.

Examine the interdependencies between these; you want to minimize points of contact. You might find that some of the classes manipulate/depend on the internals of some other class too liberally. See if you can minimize those dependencies by either moving those internals between the two classes (changing what they represent in the process), or by hiding those internals behind a method/interface that's of a higher level nature (so, it can't just be straightforward getters and setters, or methods that take the exact same parameters as the backing fields). Or, you might find methods that uncomfortably depend on another method manipulating some (local) class fields in just the right way, things like that - see if you can break that by making the method take explicit parameters instead, etc.

Finally, take a look of what you have and see if you can organize/combine these classes and/or functions further in some way that better reflects the nature of your application (or of the functionality you're trying to implement). Perhaps you can reorganize towards your ValidationService and TransformationService interfaces (or your ProcessingService interface), or even towards some other structure entirely - because now you'll likely have a better idea of what kinds of methods those interfaces should have, how many different implementations of those interfaces to create, how large or small they should be, what other code you can use to build them out of, and how you'll decide which implementation to use or inject.

This might involve a little stumbling, some trial and error, so I'd make use of git and create a local branch that I can experiment on, and safely throw away if I figure I'm not going in the right direction.