Overfullstack Overfullstack
Design

Huh? to Aha! - A Refactoring Story

As a Dev, I need to Refactor, to make the codebase Refactorable

#Java #Kotlin #Refactoring #Component Isolation #Immutability #Complexity #Testability

Abstract

As a Dev, I need to Refactor, to make the codebase Refactorable. Refactorability, as defined by Martin Fowler is, “The ability to alter internal structure without changing its external behavior”. This applies to each atomic component of an app. Let’s achieve our dev story through a unique, yet the most effective method, Component Isolation.

Let’s huddle over persuasive arguments against Obstacles that hinder our goal causing entanglements among components, and Metrics that measure our success.

Let’s refactor a real-world app and witness how Isolated components make up long-lasting structures.

Audience

This applies to software developers at all levels. I use Java to demonstrate the code snippets, but this talk is about Software Design and is agnostic of programming language.

Takeaways

Talk Outline

Let’s check out the definition from the person who authored an entire book about it.

Refactoring is a disciplined technique for restructuring an existing body of code, altering its internal structure without changing its external behavior

refactoring-def

That brings us to the story of our talk.

Goal 🎯

lego-pieces

Let’s follow the MOM process to manage our goal. Believe me, Managers love it when you do talk to them in their language. There are high chances they fund your refactoring in the same release!

Methods 🛠

We shall only focus on a couple of important Methods to achieve our goal:

Obstacles 🧗🏼

And a couple of major Obstacles that sabotage those:

Metrics 📐

And the Metrics to keep a check on our goals:

We shall apply those methods to a real-world** application** and realize how its metrics are affected.

Let’s start with Obstacles and how to overcome them:

Throw away Exceptions 👽

Mutation is Unholy 👹

Hello-real-world 🪐

Let’s see how some of what we talked about fits into a Real-world application

The Flow

flow

Partial Failures

partial-failures

void filterDuplicates(Map<ID, Failure> failureMap,
    List<Egg> eggsFromRequest) {...}


void validate(Map<ID, Failure> failureMap,
    List<Egg> nonDuplicateEggs) {...}


List<EggEntity> toEntityObjs(List<Egg> validEggs) {...}


void bulkInsertIntoDB(List<EggEntity> eggEntityObjs)
        throws DMLOperationException {...}
filterDuplicates(failureMap, eggsFromRequest);


// Partition
var nonDuplicateEggs = new ArrayList<Egg>();
for (Egg egg : eggsFromRequest) {
  if (!failureMap.containsKey(egg.getId())) {
    nonDuplicateEggs.add(egg);
  }
}


validate(failureMap, nonDuplicateEggs);
...
// Partition
...
var eggEntityObjs = toEntityObjs(validEggs);


try {
  bulkInsertIntoDB(eggEntityObjs);
} catch (DMLOperationException ex) {
  handlePartialFailures(failureMap, ex);
}


// Prepare response from `failureMap` and db insertion `results`

This flow has all the problems we discussed before. Let’s apply our methods and see how this can be Refactored into.

The Signature Shift

We can use Either to represent the partial failures. This eliminates the need for a temporary data structure like HashMap, coz now both Valids and Failures can share the same List

// Before
void filterDuplicates(Map<ID, Failure> failureMap,
    List<Egg> eggsFromRequest) {...}


// After
static List<Either<Tuple2<ID, Failure>, ImmutableEgg>> filterDuplicates(
    final List<ImmutableEgg> eggsFromRequest) {...}

partial-failures-with-either-1

partial-failures-with-either-2

static Either<Tuple2<ID, Failure>, ImmutableEgg> validate(
  Either<Tuple2<ID, Failure>, ImmutableEgg> eggToValidate) {
  return eggToValidate // No validations execute if egg is Either.left
    .flatMap(validateAge)
    .flatMap(validateField1)
    ...
    ...
    ;
}

Monad is out of scope to be covered here, but I have a blog post Monads for Drunken Coders and an entire 1 hour talk about this, which can help you fill in this missing piece:

Refactored into Isolated Pieces 🏝

This is how we refactored these components into isolated pieces.

// void filterDuplicates(Map<ID, Failure> failureMap,
//    List<Egg> eggsFromRequest) {...}
static List<Either<Tuple2<ID, Failure>, ImmutableEgg>> filterDuplicates(
    final List<ImmutableEgg> eggsFromRequest) {...}


// void validate(Map<ID, Failure> failureMap,
//    List<Egg> nonDuplicateEggs) {...}
static Either<Tuple2<ID, Failure>, ImmutableEgg> validate(
    final Either<Tuple2<ID, Failure>, ImmutableEgg> egg) {...}


// List<EggEntity> toEntityObjs(List<Egg> validEggs) {...}
static Either<Tuple2<ID, Failure>, EggEntity> toEntityObj(
    final Either<Tuple2<ID, Failure>, ImmutableEgg> egg) {...}


// Partition


// void bulkInsertIntoDB(List<EggEntity> eggEntityObjs)
//    throws DMLOperationException {...}
static Either<DMLOperationException, List<EggEntity>> bulkInsertIntoDB(
    final List<EggEntity> eggEntityObjs) {...}

But most importantly, they fit into each other, resembling a math derivation.

Let’s play some Lego

All these isolated pieces elegantly fit together, like Lego, groove-to-groove.

final var partition = filterDuplicates(immutableEggsFromRequest).stream()
  .map(EggService::validate)
  .map(EggService::toEntityObj)
  .collect(Collectors.partitioningBy(Either::isRight));


final var objsToInsert =
  partition.get(true).stream().map(Either::get).toList();


final var results = bulkInsertIntoDB(objsToInsert);
// Prepare response from `partition.get(false)` and `results`

Metrics

How are we doing on our Goal! let’s ask our Metrics:

Cognitive Complexity

I don’t need any metrics to say that the refactored code is more readable. But for a few, that might not be the case. I heard some say this is more “Complex”. Complexity is a wide word thrown around loosely. Before labeling something complex, we need to be clear to ourselves, as well as others about what is Complex.

Complexity has different types:

It has Different layers too:

The refactored code may feel complex because:

That said Cognitive Complexity can be Objectively measured using tools like SonarQube™.

Testability

If you followed the methods we discussed, you get Testability for free. Well, most of it. There are just few more that you can follow to boost it further.

Separate Static from Signal

Take the example of this simple function, which maps the request object to the entity object.

// * EggEntity is a legacy class and cannot be instantiated with `new`
void fillEntityObj(Egg egg, EggEntity eggEntity) {
  if (egg.field1() != null) {
    eggEntity.put(Fields.field1, egg.field1());
  }
  if (egg.field2() != null) {
    eggEntity.put(Fields.field2, egg.field2());
  }
  if (egg.field3() != null) {
    eggEntity.put(Fields.field3, egg.field3());
  }
}

How do you unit test all these branches? Also, because the EggEntity can’t be instantiated in a unit test context, we tend to write a test like this:

@Test
void fillEntityObjTest() {
  // * `EggEntity` Object can't be instantiated in a JUnit context.
  final var mockEggEntity = mock(EggEntity.class);
  var egg = new Egg("field1Value", null, "field3Value");
  fillEntityObj(egg, mockEggEntity);
  verify(mockEggEntity).put(Fields.field1,"field1Value");
  verify(mockEggEntity, never()).put(Fields.field2, null);
  verify(mockEggEntity).put(Fields.field3,"field3Value");
}

A Brittle Test

Make code Testable

The solution is not to write better tests but make the main code Testable. Let’s see our method closely, you can observe Static mixed with Signal:

- **Static:**  1-1 field mapping between `Egg` to `EggEntity`
- **Signal:** For each field mapping, fill non-null `Egg` fields into `EggEntity` using `put`.

Let’s do the same in code:

// Static
static final Map<String, Function<ImmutableEgg, String>> fieldMapping = Map.of(
  Fields.field1, ImmutableEgg::field1,
  Fields.field2, ImmutableEgg::field2,
  Fields.field3, ImmutableEgg::field3);


// Call with Static data and Function reference
fillEntityObj(egg, fieldMapping, EggEntity::put);


// Signal
static void fillEntityObj(
    ImmutableEgg egg,
    Map<String, Function<ImmutableEgg, String>> fieldMapping,
    BiConsumer<String, String> filler) {
  fieldMapping.forEach((fieldId, valueGetter) -> {
    final var fieldValue = valueGetter.apply(egg);
    if (fieldValue != null) {
      filler.accept(fieldId, fieldValue);
    }
  });
}

DI

Unit tests hate side-effects. All your objects used to perform side-effects should be declared as constructor dependencies or method params, so you can stub them for unit tests. Something like this:

@Autowired
public EggService(
  ...,
  @Qualifier(EGG_REPO) EggRepo eggRepo,
  ...
)

But if all you need is a function from a Dependency, resist injecting the entire object. Instead, inject only the function that you need.

@Autowired
public EggService(
  ...,
  // @Qualifier(EGG_REPO) // EggRepo eggRepo,
  @Qualifier(EGG_INSERTER) Function<EggEntity, ID> dbInserter,
  ...
)


// Main Config
@Bean(EGG_INSERTER)
public Function<EggEntity, ID> insert(
  @Qualifier(EGG_REPO) EggRepo eggRepo) {
  return eggRepo::insert;
}


// Test Config
@Bean(EGG_INSERTER)
public Function<EggEntity, ID> insertNoOp() {
  return eggEntity -> new ID(1); //stub
}

This helps in easy stubbing of this dependency without the need for a mock, and we get the same benefits that we discussed in Separate Static from Signal

Testability First, Test Coverage follows 🐕

Notice, I use the word Testability over Test Coverage. Testability indicates how precisely you can hit a component without the need to mock a lot of things or know its internals. Test coverage is just a metric. It’s very much possible to achieve 100% Test coverage without your code being testable. Such synthetic coverage of course is useless.

Last but not the Last

Test 🏓 Refactor

Tests should not be an afterthought and should happen parallel to Refactor, like ping-pong.

Entropy

Even bad code can function. But if the code isn’t clean, it can bring a development organization to its knees - Uncle Bob, Clean code

Resources

My Talks on this

Back to all articles