Tag Archives: code

Finish something every day

When you write code in an engineering organization, you will do the following:

  • Type the code out.
  • Test some of it. Hell, maybe you’ll test all of it.
  • Get someone to review the code.
  • Push it to source control.

These items aren’t discrete or ordered. Test-driven development and pair programming are practices that reorder or merge these items. But these should happen for most changes.

Sometimes, you’re given a large task. You have a question at this point: should I break it up? Should I write the whole thing at once? In my experience, the best tradeoff is to finish something every day, even if it’s small. Write it. Test it. Send it for review.

This introduces a lot of tradeoffs. It’s not always possible. It makes some assumptions about your work environment. We will discuss all of these below.

Benefits

Small changes are better for you

Let’s say that you’re a full stack developer at a web shop. You are assigned a new task: add a new form to collect and store some data, and then display it on another part of the site. The designer whipped up mocks. The product manager wrote out the expected behavior. Now it’s your turn.

You see all of your tasks: Modify a form to collect the data. Include it on the request to the server. Write it into the ORM. Modify the database to store it. Think about the default values and whether we can backfill the data from another location. Read the data from the new location. Render it into the view.

There are a few obvious ways to approach the code:

  • Write all of it at once.
  • Write the database code, write the write path, write the read path.

There’s a less obvious way to approach the code:

  • Write the database code, write the read path (with stubbed data), and write the save path.

There may be other alternatives that depend on the details of your project. But look at what happened: The project is now decomposed into smaller tasks. Even better, we see that the ordering of two of the tasks doesn’t matter. The data layer code blocks everything else. But the other two tasks are independent of each other. You can pick the most convenient task ordering. They could even be done at the same time. This is the first insight of decomposing tasks: some work becomes parallelizable.

Parallelization is where the magic happens. This means that you’ve converted a one-developer project into a two-developer project. This is a great way to grow as an engineer. It lets you practice for project lead or tech lead positions. This also helps you practice for artificial deadline pressure. In an “emergency,” you could make the following proposal to your team: “we can get this out faster if we add a second engineer to this. I can add the model code today. Someone can work on the view tomorrow while I work on the save path.”

It’s also good to practice to go through a full “write, test, review, deploy” cycle frequently. Practice makes perfect. You will become more capable as you push more and more code. Your “small” changes will become larger and larger. It also becomes insurance against seniority. As you get more responsibilities, you will probably suffer from the Swiss cheese calendars that plague many senior employees. It’ll be your job to help people and maintain relationships around the company. People often need help at awkward times on your calendar. If you are in the habit of producing small changes, it’s a little easier to write code. You can still finish something if you have two hours between meetings.

Interestingly, you will discover failure cases as you parallelize work. These failure cases aren’t always obvious. What could go wrong? Some tasks are just too small. Every “write, test, review, deploy” cycle has overhead. Sometimes the overhead is huge compared to the size of the change. You will also notice that saturating a project with the maximum number of engineers doesn’t work as well as it sounds. If someone’s schedule slips, other engineers will be blocked. This is okay in the occasional “emergencies” where shipping a feature ASAP is the most important thing in the world. But you burn intangible resources (goodwill, happiness, team cohesion) by perpetually oversubscribing projects. You will learn to find a sustainable headcount for a project.

There are selfish reasons to ship all the time. Shipping is a form of advertisement. People see that you’re constantly “done” with something because you’re always asking for a review. But this is a double-edged sword. You’re always going to be asking for code reviews. The reviews should be worth the time of the reviewer. Make them large enough to be interesting. If you’re distracting them with adding a single line, you’re doing the team a disservice. This is why I’ve found “a day of work” to be a good tradeoff.

Better for the codebase

I’m going to tell you a horror story. Remember the above example: adding a UI feature to a web app? I’m going to work on that change. And I’m going to do the whole thing in a single pull request. I swoop my cape over my face and disappear into my evil lair for over a week.

I send you the code review out of nowhere. You look at it. Thousands of lines added across a few dozen files: tests, database configurations, view templates. This is going to take forever to review. You skim the files. You eventually get to the database file. You see that something is wrong: I should have added another table for the new data. Instead, I wedged it into an existing record. And this change was foundational. The write path depends on this mistake. The read path depends on this mistake. The UI on either side depends on this. The tests assert this. Everything depends on this mistake. Fixing this is expensive. It’s closer to a rewrite than a refactoring.

But we’re in the “website economic model” of development. Our sprint process puts downward pressure on how much time this task should take. I shipped a functional version of the project. It’s now your job to argue that we should throw away a working version in favor of a different working version.

This puts you in a difficult spot. The team estimated this task would be completed in under 1 sprint. But now we’re more than halfway to the deadline, and the change is wrong. Fixing it will take it past the end of the sprint. I’m incentivized to push back against your feedback. I may not. But let’s remember: this is a horror story. I’m going to push back. Bringing this up will also invite discussions with product or management stakeholders to negotiate whether there’s a cheaper fix that avoids a rewrite.

Furthermore, it took you forever to review the entire change. You need to do the entire review again a second time after my rewrite. And maybe a third time if another round of revisions are necessary. That could up to hours of reviewing that you’re not dedicating to your own work.

All of this leaves you with two bad options: rubber stamping a bad change (with some perfunctory “I was here” feedback to show that you reviewed it), or reducing your own velocity and your team’s velocity to argue for a gut renovation because of less-tangible long-term improvements.

Ok, let’s end the horror story. What if I had split my task into day-long chunks? My first task would be to write the data layer. So I’d write the database changes and any ORM changes. I’d send them to you for review. You’d look at my changes and say, “Hey, let’s move these fields into a separate table instead of wedging this into the HugeTable. We used to follow that pattern, but we’ve been regretting it lately for $these_reasons.” And it’s totally cool – I don’t push back on this. I take the few hours to make a change, you approve the changes, and I move on.

What was different? I incorporated you earlier into the process. You weren’t sacrificing anybody’s time or velocity. You made me better at my job by giving me feedback early. The codebase turned out better. Nobody’s feelings were hurt. This means that I improved the entire team’s engineering outcome by splitting my changes into small chunks. It was easy to review. It wasn’t difficult for me to fix my mistakes.

Why wouldn’t I split my changes into smaller chunks?

When does shipping every day work? When does it fail?

“Finish something every day” makes a lot of assumptions.

There is an obvious assumption: something worthwhile can be finished in less than a day. This isn’t always true. I’ve heard of legacy enterprise environments where the test suite takes days to run. I’ve also worked in mobile robotics environments where “write, compile, test” cycles took 30 minutes. In those situations, it can be impossible to finish something every day. There is a different optimal cadence that balances the enormous overhead with parallelization.

“Finish something every day” also assumes that the work can be decomposed. Some tasks are inherently large. Designing a large software system is a potentially unbounded problem. Fixing a performance regression can involve lots of experimentation and redesigning, and is unlikely to take only one day. Don’t kill yourself trying to finish these in a day. But it can be interesting to ask yourself, “can I do something quickly each morning and spend the rest of the day working on the design?”

Another assumption is that your teammates review code quickly. Quick reviews are essential. This system is painful when your code reviews languish. Changes start depending on each other. Fixes have to be rebased across all of them. Yes, the tools support it. But managing 5 dependent pull requests is hard. Fixes in the first pull request need to be merged into all the others. If your teammates review them out of order, fixing all of them becomes a nightmare.

If I may be so bold: if you’re getting slow code reviews, you should bring it up with your team. Do you do retrospectives? Bring it up there. Otherwise, find a way to get buy-in from your team’s leaders. You should explain the benefits that they will receive from fast code reviews: “Fast code reviews make it feasible to make smaller changes, because our work doesn’t pile up. Our implementation velocity improves because we’re submitting changes faster. We all know that smaller changes are easier to review. It’ll lead to better engineering outcomes because we’ll provide feedback earlier in the process.” Whatever you think people care about. Ask your team to agree on a short SLA, like half a day.

You can model the behavior you want. You should review others’ code at the SLA that you want. If you want your code reviewed within a couple of hours, review people’s code within a couple of hours. This works well if you can provide good feedback. If you constantly find bugs in their code, and offer improvements that they view as improvements, they’ll welcome your reviews and perspective as critical for the team’s success. If you nitpick their coding style and never find substantial problems, don’t bother. The goal is to add value. When you’re viewed as critical for the team’s success, then it’s easier to argue that “we will all be more successful if we review code quicker.”

I take this to an extreme. When I get a code review, I drop everything and review it. My job as a staff engineer is to move the organization forward. So I do everything possible to unblock others. If this doesn’t work for you, find a sustainable heuristic. “Before I start something, I look to see if anyone has a pending code review that I can help with.” Find a way to provide helpful code reviews quickly.

Finish something every day

Try to finish something every day. You will get better at making small changes, and your definition of “small” will keep getting bigger. You will get better at decomposing tasks. This is the first step towards creating parallelizable projects. Additionally, you will get exposure for continually being “done.”

It helps your reviewers. Smaller tasks are much easier to review than larger tasks. They won’t have to give large reviews. They also won’t have to feel bad about asking for a complete rewrite.

It will also help the codebase. If reviewers can give feedback early, they can help you avoid problems before you’ve written too much code to turn back.

In practice, “finish something every day” really means “find the smallest amount of work that makes sense compared to the per-change overhead.” In many environments, this can be “every day,” but it won’t be universal.

Please consider donating to Black Girls Code. When I was growing up,
I had access to high school classes about programming and a computer
in my bedroom which allowed me to hone these skills. I'd like
everyone to have this kind of access to opportunities.

https://www.blackgirlscode.com/

I donated $500, but please consider donating even if it's $5.

Simple Software Engineering: Use plugin architectures to improve encapsulation

Let’s say that you have to write an API service. The service handles different types of API calls – calls coming from third parties, your company’s apps, and your company’s internal servers.

There will inevitably be call sites that behave differently for different services. Imagine some examples: third-party services use a different authorization method, internal-only responses get debugging headers, error payloads are formatted differently for apps. They all use different loggers. The list goes on.

By the end of implementation, many call sites will have logic that is conditional on the service type.

// This method has two different call sites that depend on the service:
// logging and the authorization handler.
public function handleAuthorization(Service $service, Request $request) {
    $auth = ($service->type === Service::TYPE_THIRD_PARTY)
        ? new Service\Auth\ThirdParty()
        : new Service\Auth\SomethingElse();

    $logger = null;
    switch($service->type) {
    case Service::TYPE_THIRD_PARTY:
        $logger = Service\Logger\ThirdParty();
        break;
    case Service::TYPE_APP:
        $logger = Service\Logger\App();
        break;
    case Service::INTERNAL:
        $logger = Service\Logger\Debug();
        break;
    default:
        throw new Exception('Unrecognized logger');
    }

    try {
        $auth->check($request);
    } catch (Exception $e) {
        $logger->logError($e, [/* some data */]);
        throw $e;
    }

    $logger->logInfo('successful authorization', [/* some data */]);
}

When many call sites behave differently based on the same set of conditions, this can be considered a “two dimensional problem.” I don’t think there’s an official definition to this. I just like to think about it this way. One dimension comprises each of the different conditions. The other dimension comprises all of the call sites depending on the service type. In the API service example, the service dimension has…

[third-party API calls, calls from company apps, calls from internal servers]

and the call site dimension has… 

[Handling authorization, header response selection, response formatting, logging]

In this example, there are 12 combinations to be considered (three origins * four call sites). If a new call site is added, there are 15 total considerations. If a new origin is added after that, there are 20 considerations. This will likely grow geometrically over time.

It’s tempting to say, “This example should be dependency-injected anyways.” But this is just a demo of the problem. Dependency injection doesn’t solve the real problem, which is that the service definitions have no cohesion. The service is a first-class concept within the API. But the definition of each service is scattered throughout the codebase, which leads to some problems.

Switching on types is error-prone across several usages.

Writing cases manually is error-prone. When adding a new type, the author must vet every call site that handles types. The new type might need to be specially handled in one of them. These call sites can be hard to enumerate: it could include all locations where any of the types are checked. Even worse, the list can include sites where the logic works because of secondary effects. For instance, “if this logger type also implements this other interface, do this other logic” might be attempting to define logic for the single service that provides that interface type.

Let’s say that the whole API team gets hit by a bus. It’s sad, but we must increase shareholder value nonetheless. The old team began a new project: adding an API service handling our new web app! So the replacement team defines authorization and response logging and launches the service into production. But they missed a few cases. A few weeks after launch, the new web service is down for two hours and no pages were fired. After some investigation, it turns out that the wrong logger was used and the monitoring service ignored errors from unrecognized services. Later, the company pays out a security bounty because internal-only debug headers were leaked. These are plausible outcomes of dealing with a low-cohesion definition – because the entire definition can’t be considered at once, it’s easy to overlook things that cause silent failures.

Switching on types has low cohesion.

When logic depends on the same conditions throughout the codebase, the cohesion of that particular concept is low or nonexistent. This makes sense: the service’s definition is scattered throughout the codebase. It would be better if all of these definitions were grouped behind the same interface. This makes it easy to describe a service: a service is the collection of definitions inside an implementation of the interface.

Prefer plugin architectures

Me in front of the painted ladies in San Francisco
Marveling at the Painted Ladies on a recent trip to San Francisco. An obvious example of plugin architecture if I’ve ever seen one.

What does the code example look like within a plugin architecture?

// Provides a cohesive service definition.
interface ApiServicePlugin {
    public function getType(): int;
    public function getAuthService(): Service\Auth;
    public function getLogger(): Service\Logger;
    public function getResponseBuilder(): Api\ResponseBuilder;
}

// Allows per-service objects or functions to be retrieved.
class ApiServiceRegistry {
    public function registerPlugin(ApiServicePlugin $plugin): void;
    public function getAuthService(int $service_type): Service\Auth;
    // Not shown: other getters
}

public function handleAuthorization(Service $service, Request $request) {
    // Note: These would likely be dependency-injected.
    $auth = $this->registry->getAuth($service->type);
    $logger = $this->registry->getLogger($service->type);

    try {
        $auth->validate($request);
    } catch (Exception $e) {
        logger->logError($e, [/* some data */]);
        throw $e;
    }

    $logger->logInfo('success', [/* some data */]);
}

The plugin interface improves cohesion.

The plugin provides a solid definition of an API service. It’s the combination of authorization, logging, and the response builder. Every implementation will correspond to a service, and every service will have an implementation.

Plugins enforce that all cases are handled for each new service.

It’s impossible to add a service without implementing the full plugin definition. Therefore, every single call site will be handled when a new service is added.

Adding a new call site means that every service will be considered.

When adding a new call site for the service, there are two options. Either it will use an existing method on the plugin interface, and all existing services will work. If a new concept needs to be added to the plugins, then every plugin will need to be considered.

Plugin registries make testing much easier.

Plugin registries provide an easy dependency injection method. If a plugin is not under test, simply register a “no-op” version of the plugin that does nothing or provides objects that do nothing. If something shouldn’t be called, simply provide objects that throw exceptions when they are called. Because each of the call sites are no longer responsible for managing a fraction of the service, the tests can now focus on testing the logic around the call sites, instead of partially testing whether the correct service was used.

Avoid plugin registries when one of the dimensions is size one

Registry configs are great for reducing dimensionality. But what if there is either just a single service, or just a single call site? Then it would be overkill to make the full class and interface hierarchy. If there is just one call site, then write the basic switch statement or if/else chain. If there is only one mapping type that is being shared across a few call sites, then refactor it into a map or a helper function. The full plugin architecture is only useful when managing the complexity of many services used at many call sites.

Simple software engineering: Inject dependencies when possible

Why should dependencies be injected?

Code should not instantiate or otherwise access its own dependencies. Instead, prefer to pass in dependencies as arguments.

This should be done when code becomes important enough to unit test. Dependency injection makes it easier for tests to provide dependencies with different configurations. It also makes it easier to inspect the side effects introduced onto dependencies. It will also make maintenance easier because it will increase flexibility at little cost.

When working with I/O, pass in interfaces instead of instantiating classes

// Avoid new()ing the dependency
public function getDatabaseConnectionInfo(): ConnectionInfo {
    $redis = new Redis();
    return new ConnectionInfo(
        $redis->get('host'),
        $redis->get('port')
    );
}

// Prefer to pass the dependency in
public function getDatabaseConnectionInfo(
    Redis $redis
): ConnectionInfo {
    return new ConnectionInfo(
        $redis->get('host'),
        $redis->get('port')
    );
}

// Passing in an interface is even better
public function getDatabaseConnectionInfo(
    KeyValueInterface $key_value_store
): ConnectionInfo {
    return new ConnectionInfo(
        $key_value_store->get('host'),
        $key_value_store->get('port')
    );
}

Dependency injection makes testing easier.

The unit test can take a trivial in-memory store instead of either wrangling a Redis instance or mocking a constructor.

Additionally, code that manages its own dependencies can become nested deep within other code. If the first example above became deeply nested, it would be unclear that the corresponding test must manage a key/value store. This could lead to bad surprises like tests attempting to connect to Redis.

Dependency injection increases flexibility at little cost.

In the example above, it may become necessary to introduce a caching layer in front of the key/value store. This is easy with dependency injection. Just make a new class that inherits the interface and delegates to the I/O layer on cache misses. Without dependency injection, it becomes a project to find and fix all usages.

Dependency injection makes it easier to make application-wide changes.

In the above example, it may become necessary to stop using the default Redis database and configure which Redis database should be used. If the Redis class is instantiated in many places, this becomes a sizable effort. Compare this to changing the single invocation that is injected throughout the application. The latter will usually be much easier.

Pass the result of I/O into business and presentation logic

// Avoid performing I/O in business logic
public function isShopTemporarilyClosed(
    ORM $orm, int $shop_id
): bool {
    // All shops manually turned off by the owner
    // are called temporarily closed.
    $shop = $orm->getFinder('Shop')->findById($shop_id);
    return $shop->is_off
        && $shop->owner->id === $shop->disabled_by_user->id;
}

// Prefer passing the result of I/O into business logic
public function isShopTemporarilyClosed(
    Shop $shop
): bool {
    // All shops manually turned off by the owner
    // are called temporarily closed.
    return $shop->is_off 
        && $shop->owner->id === $shop->disabled_by_user->id;
}

Business/presentation logic should not be overly opinionated

Applications often have several choices about where they can read equivalent data. This function shouldn’t care that the data came from the ORM. Why couldn’t it be passed in the POST data of a request? Or be fetched from a REST API? Ideally, logic that acts on a shop model should work anywhere.

Doing I/O in application logic makes its callers difficult to refactor

As a codebase grows, helper functions may acquire dozens or hundreds or thousands of callsites. They may become nested deep within the application call stack. It will be used within business-critical logic that will run into scaling problems. Manually managing dependencies makes it difficult to perform some optimizations. For example, it’s difficult to ensure that the program never makes redundant I/O calls when the object is accessed via I/O dozens of times in a codebase. This is true even with caching! It’s often the case that calling two different I/O entry points (or the same entry point with different arguments) can produce the same results. This can be difficult or impossible to programmatically detect, even though it may be obvious to the application developer.

Passing in the result of I/O makes it easier to share the result of I/O among different callers.

I/O introduces nondeterministic behavior
It would be surprising to see a DatabaseReadException when calculating whether a shop is closed. But introducing I/O into a call increases the risk that code can throw exceptions for nondeterministic reasons like service availability.

I/O also dramatically affects timing metrics. Let’s say that I/O calls are cached, and the shop is fetched in two places: once while deciding which views to render, and once while rendering the view. Later, a programmer realizes that they don’t need to perform the first fetch. They remove it. This will move the I/O call from the application logic into the view logic, causing the view logic’s timing instrumentation to increase. This is because a former cache hit is now a cache miss with I/O fetching. No regression happened, but the application performance graphs make it seem like one did.

This could also cause tests to become flaky if they actually perform the I/O and sometimes fail.

Instead, prefer to centralize or share logic related to I/O. The details of this will depend on which languages and libraries are used.

Don’t access static or global state within business or presentation logic

// Avoid accessing global or static state in business logic
public function isLocaleEnUs(): bool {
    return strtolower($_REQUEST('locale')) === 'en-us';
}

// Pass global or static state into business logic
public function isLocaleEnUs(string $locale): bool {
    return strtolower($locale) === 'en-us';
}

Accessing static and global state is unsafe across machines

A helper that access static or global state makes strong assumptions about what happened on the machine prior to the code executing. Note: this refers to static state – the reliance on information from the execution environment, or calculated data that is stored statically. Accessing static data or static functions isn’t included in this.

If code lives long enough, it will eventually execute in several layers of the same application stack. Think about all the different application architectures that can exist in the same company at the same time. Reverse proxies in front of long-lived application servers, CGI scripts, batch processing jobs, monoliths, microservices, single-page applications, mobile apps, serverless lambdas, server-side rendering, etc. And to add another dimension, there are quite a few transport mechanisms available: HTTP, RPC, IPC, etc.

As code becomes longer-lived, it will eventually live within several layers at the same time. This introduces unnecessary complexity on each of the additional layers. If some logic directly reads the request parameters to determine the locale, that it (and every thing that ever depends on it) will always need to execute within an HTTP request. Or it must fake the HTTP request environment when it is included in a layer without HTTP. Or if it’s proxied within HTTP, the proxied call will also need to forward the request parameters, even if it doesn’t make semantic sense.

How to move an existing codebase towards dependency injection

This can be done incrementally. For each commit that uses a dependency, refactor that dependency to come from one layer higher in the stack. This is a good opportunity to introduce tests for untested code, or simplify tests for existing code.

Over time, frequently-modified code will become fully implemented using dependency injection. It may be necessary to do a special project to modify, replace, or delete code that hasn’t been touched in years. But maybe it’s fine to just leave it. After all, it hasn’t been modified in years.