Let me be a feedback loop for you

(this post was written with the intent to give it to my current, and future, direct reports in an attempt to establish part of our relationship)

Every time we need to solve a problem (or implement a feature) the process is the same

  1. First we think the approach we are going to follow. This is an abstract flow that we believe will solve the problem.
  2. Based on that we divide our approach to one or more components (modules / classes / functions, whatever suits the size of the problem) and assign a certain behavior to each one of them.
  3. Finally, we start implementing each component.

Feedback loop

How do we know if something we implemented is correct?

By getting feedback. Based on it we make improvements and try again. This loop keeps us on track and allows us to deliver something valuable.

This is why we try to have frequent and short feedback loops. This way any corrections occur sooner than later and we don’t spend time and effort into something that might, potentially, thrown away.

Tech lead as a feedback loop

Looking back to the process of solving a problem we could say that two out of the three steps do have a feedback loop to helps us.

In the implementation step we can argue that the compiler / linter / etc gives us immediate feedback on what we wrote. We fix it and move forward.

For the behavior step we get the feedback through tests. Is the component’s behavior the expected one? How about the components API? Can we use it easily in our tests? Answering these question helps in having solid components.

But what about the approach step? This is where the tech lead comes in.

Right before writing any code, thus putting effort into something, prepare a simple list with the steps you are about to take and discuss them with your tech lead. If there is any feedback use it to improve your approach and have another meeting.

Is it time consuming?
No. In case the approach needs improvement after the implementation the time to fix it will be longer.

Do you feel bad on spending your TL’s time?
Don’t. It is, literally, their job to help you.

Add tests, discover things!

The benefits that you get from testing your code will never cease to amaze me.

I was working on a new feature and part of it required changing one of our oldest classes. The class was still in Java so I decided, before doing any other work, to convert it to Kotlin. There were no tests so my first step was to add them and make sure that the conversion wasn’t going to break anything.

The class is a simple configuration one that

  • setups a couple of its fields upon construction and
  • exposes its validity state

something like this

public final class Configuration {
private final String agent;
private final String header;
private final String version;
public Configuration(final Storage storage) {
this.agent = storage.get(KEY_AGENT);
this.version = storage.get(KEY_VERSION);
this.header = storage.get(KEY_HEADER) + "/" + VERSION_PREFIX + this.version;
}
public boolean isValid() {
if (getAgent().length() == 0) return false;
if (getVersion().length() == 0) return false;
return getHeader().length() != 0;
}
public String getAgent() {
return agent;
}
public String getHeader() {
return header;
}
public String getVersion() {
return version;
}
}

but with many more fields.

Falsely valid

So I started writing tests to cover all cases and the one that failed immediately was

@Test fun `the configuration is not valid when its header is empty`() {
whenever(mockStorage.get(KEY_AGENT)).thenReturn("an agent")
whenever(mockStorage.get(KEY_VERSION)).thenReturn("a version")
whenever(mockStorage.get(KEY_HEADER)).thenReturn("")
val actual = configuration().isValid
assertFalse(actual)
}

at first I thought I had an error in the test but looking closely at the production code I saw the problem. Actually the problems. Plural:

  1. Even thought in this case its subtle, the constructor is violating the SRP. It initializes the fields and makes a decision on what will be exposed. This is a job for the getter.
  2. Which brings us to the second problem. Getters are great way to expose information allowing us to make internal changes without breaking anything outside the class. They help us decouple ourselves from the classes that consume us.
    By using the getters instead of the fields, as in isValid, we couple the class with itself making it depend on what will be exposed and not the actual internal state of the class.

For the record the fix is

public final class Configuration {
private final String agent;
private final String header;
private final String version;
public Configuration(final Storage storage) {
this.agent = storage.get(KEY_AGENT);
this.version = storage.get(KEY_VERSION);
this.header = storage.get(KEY_HEADER);
}
public boolean isValid() {
if (agent.length() == 0) return false;
if (version.length() == 0) return false;
return header.length() != 0;
}
public String getAgent() {
return agent;
}
public String getHeader() {
return header + "/" + VERSION_PREFIX + this.version;
}
public String getVersion() {
return version;
}
}

and we managed to figure it out by writing a small, simple test.

Quick feedback

Having a quick feedback loop is a great way to keep yourself focused on the end result and to make less mistakes on the way. In our profession this is achieved with tests.

This bug was hiding for quite some time so the loop in this implementation was literally years!
Aim for shorter time periods! Seconds is the best and to do that is to write the test along side the production code.

I strongly encourage you to follow TDD but it doesn’t matter if its not your cup of tea. Write it after the production code, just do it immediately after.