Do Ya Still Need It

Most of us are familiar with YAGNI (“You Aren’t Gonna Need It”), that old developer mantra reminding us not to add code or functionality until it’s truly needed. But if we have a mantra for not adding unnecessary code, it seems only fitting we have one for removing code that’s outlived its usefulness. That’s where my internal monologue often kicks in: DYSNI - Do Ya Still Need It?

Ruthless Simplification with DYSNI

At the day job, my colleagues Adam and Ellen (aka the only two who foolishly raised their hands when I asked for volunteers to help remove some Objective-C) and I were doing some Objective-C cleanup.

We still have a portion of Objective-C in our codebase, but we’re aiming to remove it. The team’s Objective-C skills aren’t being exercised and risk atrophying, new hires rarely have experience with it, and its presence complicates adopting modern technologies such as Swift Concurrency.

The first instinct when migrating from Objective-C is usually to just port the code like for like. There’s often an attempt to make things feel more Swifty, but ultimately the original structure tends to linger.

By asking DYSNI on repeat, we were able to take a pretty gnarly abstraction and simplify it right down.

The Problem

The starting point we had was this lovely structure

     +------------------------+
     |      CommandBlock      |
     +------------------------+
     | +executeWithCompletion |
     +------------------------+
        ^                  ^
       /                    \
      /                      \
+----------+            +----------+
| CommandA |            | CommandB |
+----------+            +----------+

The signature of the completion handler was void (^)(NSDictionary *userInfo, BOOL success, NSError *error) (don’t we all just miss the Objective-C syntax?). The way this was being used is that there was a view controller that held a reference to the current command

@property (nonatomic, strong) CommandBlock *currentCommandBlock;

The currently executing block would be stored in this ivar until it completed its work and then it was nil‘d out.

The first DYSNI

We started in possibly the worst place but hey that’s how reality is sometimes. We asked Do Ya Still Need It whilst looking at this hierarchy. The thinking was that if you have a class with a single method, it could be represented as a single anonymous function. So instead of holding onto the CommandBlock instance itself we could just hold on to the function reference execute(completion:) with a view to removing the base class at some point.

Why was this a bad starting point? Because we had to remember Objective-C block syntax… on the second attempt, we got it working and made the following change

- @property (nonatomic, strong) CommandBlock *currentCommandBlock;
+ @property (nonatomic, strong) void (^action)(void (^)(NSDictionary *userInfo, BOOL success, NSError *error));

Just as we were about to update everything, we paused and asked DYSNI again…

The second DYSNI

Before updating all the code, we wondered if we should check all the callers to see what userInfo is even being used for. Upon tracing things through for a bit we found that precisely zero callers were using userInfo, so we made the change

- @property (nonatomic, strong) void (^action)(void (^)(NSDictionary *userInfo, BOOL success, NSError *error));
+ @property (nonatomic, strong) void (^action)(void (^)(BOOL success, NSError *error));

That worked well and we kind of assumed that success and error would be used. Thankfully that assumption didn’t stop us asking DYSNI.

The third DYSNI

We questioned who’s using error so we did the same dance but this time we found one caller was actually reading the error. Out of curiosity, we followed that caller through and found they didn’t actually do anything with error so

- @property (nonatomic, strong) void (^action)(void (^)(BOOL success, NSError *error));
+ @property (nonatomic, strong) void (^action)(void (^)(BOOL success));

We had a flow going so you guessed it…

The fourth DYSNI

We had to ask is anyone using success or do callers just want to understand when the operation has finished? In this case success is required so we couldn’t delete it 😢 but never fear because we made things much simpler already. Thanks to those small DYSNI checks, no future readers of the code would need to ask the same questions we just have.

The fifth DYSNI

At this point we’d spent a fair bit of time in the weeds looking at this function so we zoomed out a bit and asked are these commands even required at all? What we found was that at some point all uses of CommandA had been removed from the codebase but this was never tidied up. This unlocked many more opportunities to simplify, for starters we got our delete on and went from

     +------------------------+
     |      CommandBlock      |
     +------------------------+
     | +executeWithCompletion |
     +------------------------+
        ^                  ^
       /                    \
      /                      \
+----------+            +----------+
| CommandA |            | CommandB |
+----------+            +----------+

to

+----------+
| CommandB |
+----------+

The sixth DYSNI

With this simplification done we went back and asked the question we didn’t want to ask but knew we should, do we still need the function signature we spent ages messing around with? The answer was no we do not - it turns out that we don’t know why there was an instance variable in the first place. Our best guess is that the original developers assumed they needed to keep a reference to the command whilst it was executing and then nil it out on completion. In reality, the command is a one shot deal that keeps itself alive until the work is completed then it would naturally go out of scope.

With this knowledge we said goodbye to the ivar

- @property (nonatomic, strong) void (^action)(void (^)(BOOL success));

Instead we just new up an instance of the command and invoke it

[CommandB.new executeWithCompletion:^(BOOL success) {
    // do some stuff
}];

*Controversial use of dot syntax on the new there 👀.

Conclusion

Asking the question “Do Ya Still Need It?” can be a surprisingly powerful tool. The story above resulted in a diff of 126 insertions(+), 928 deletions(-), which is a great result. Not only did we reduce line count, but we also encoded our new understanding of the problem into more modern approaches, which will hopefully be easier for future readers to pick up.

Sometimes, when I ask DYSNI, I get the sense people think I’m being lazy or annoying on pull requests but ruthlessly simplifying things down means less work now and less complexity to unpick later.