In Java, there are two types of exceptions:

• Checked — these exceptions are checked by the compiler. When you throw it, you need to catch it in a try/catch block or mark the method with a throws declaration, otherwise, your code won’t compile. Checked exceptions are the ones derived from the Exception class.
• Unchecked — these are not checked at all at compile time. You could use them without a try/catch block or even a throws declaration. If you want to create one you must derive it from the RuntimeException class. The Java runtime itself also defines unchecked exceptions from the Error class. These could be considered as a third type but we don’t throw them.

Most of the other languages don’t have a similar concept like the checked exceptions, because sometimes it feels like a burden to deal with them, but let’s look at these types and when should you use them!

Unchecked exceptions

These exceptions are the ones that you could prepare for. If they occur it’s your fault. But why am I saying this?

Let’s see a few of them:

• NullPointerException: This one is my favourite, it happens when you invoke a method or try to access a property on a reference that’s basically empty. But you could’ve checked that before. So when a NullPointerException occurs it’s mostly because you were lazy to check it or make sure that it won’t happen.
• ArithmeticException: Another nice one. It occurs when you do something nasty in terms of arithmetic, like when you try to divide something with zero. I can’t repeat myself enough: why on earth haven’t you checked that the divisor is zero?
• IndexOutOfBoundsException: This happens when you try to access an index which is out of the bounds of the container (a string or an array). Shame on you!
• UnsupportedOperationException: Well, this is kind of special in terms it’s not because you personally screwed up something. It’s because the Java runtime creators tried to make backward compatible changes, and therefore they screwed up the Collections API with something that just not fit. Except you throw this one by hand because then you are violating the ISP and.. you know.

So the solution for this kind of exceptions? Precautions.

Regarding the Errors: you shouldn’t try to catch them! Just NO. These indicate bigger problems than you could handle inside from the JVM.

Checked exceptions

What about the checked exceptions? You just can’t prepare for these. They happen because of an external reason.

• IOException: Ouch. We might think that network is always there, our hard disk is granted, no-one else deletes the files we are about to read, but sometimes.. well, sh*t happens.
• SQLException: Ah, we almost forgot our arch enemy. We encounter this when the query is messed up, the table contains different type we are trying to extract and so on. There is no method for accessing SQL in a peaceful way. Exception or ResultSet. You choose.
• ScriptException: Jesus, why do Java even have this? This happens when an underlying scripting interpreter fails to interpret or run some scripts. Again, how could you test if some JavaScript snippet contains syntax or runtime errors without actually interpreting or executing it? Yes, Regexp! No.

So we ended up without proper precautions to be taken for the problems which might cause the problems listed above. Therefore we should follow this idea and use checked ones when you just can’t provide a way to create a foolproof solution. Every other case you should stick to the unchecked ones and derive from the RuntimeException class.

Another concern against the checked ones if you use them extensively for no reason, then the user of your framework/lib will end up swallowing these in his code, scattering around empty catch blocks, because he just can’t deal with the exceptions which propagate through the control flow. Also these appear in the source code, therefore it creates a compile-time dependency and that’s against the dependency inversion principle.

So next time, when you create an exception in Java, stop for a minute and ask yourself a question:

• Do the client of this method could do precautionary measures in order to avoid this error? Yes? Make it unchecked.
• This problem happens because of external factors like I/O or arbitrary parameters and it’s cannot be predicted? Make it checked.

Let’s look at those benefits!

• Better design — it’s kind of obvious because in order to write a unit test for a particular unit, you’ll need to design it in a way that it’s easy to decouple from the other parts of the code
• Documentation — the test names document how that unit works. Even with a badly named method you could figure out what the test does by looking at its body
• Finding bugs — even though it’s not what they are for, tests could help finding regression bugs
• Reducing costs in a long term — For a project that lasts more than a few weeks and is more complex than a landing page, tests will help you avoid making a mess which would result in extra man hours and therefore spending extra money.
• Ability to safely refactor — this will be the focus of this post.

What is refactoring?

Code refactoring is the process of restructuring existing code, without changing its external behavior.

What do we mean by “without changing its external behavior”? It means that we select a chunk of code, and we modify it without changing is pre- and postconditions. It will do the exact same thing it did before. In case that chunk is a method, it means that its interface doesn’t change, only its body.

How do we make sure nothing changed? Well, that’s where tests come into the picture.

If we had tests for that code chunk, we could just run them again and any test failure would be a sign that we’ve changed their behavior.

Refactoring vs new features

Refactoring is the process of restructuring the code while intentionally not modifying the behavior.

Adding a new feature is the process of intentionally modifying the behavior.

Most of the time people mix up adding a new feature with refactoring existing code. This process often doesn’t stop at one class, but with rewriting a bunch of code possibly making things even worse. At the end the tests are run and a lot of them fail. What could be the cause? Is it the refactor? Or is it the logic added to handle that new feature which somehow doesn’t fit into the system? Well.. who knows? Therefore people end up either reverting these changes or — worse case — updating the tests to reflect the new outcome even if they don’t understand it.

I would advice against both of it. Just hack your way through until your feature works — covered with tests of course — and then start to clean up the mess you made.

At the end of one of my presentation somebody from the crowd asked me if he had done something wrong because:

A bunch of test broke when I refactored the code, so I had to update them as well…

Well it’s kind of a different story than what we spoke about above because this time it’s pure refactoring and not mixing refactoring up with new features.

So then what could be the cause? The fact that he said “a bunch of test” made me wonder about how does he structured his tests? It could be because he rewrote half of his system — so the scope of the refactor was too big — or because he had done some bad things in his tests like creating them too generic — we will dig into this later.

At least he didn’t think that this is normal, because unfortunately there are a lot of developers who do.

What could be a reason for all of this?

• Misconceptions of refactoring
• Definition of unit
• Fragile test problem(s)

Let’s discuss these one by one!

Misconceptions of refactoring

Although this post is not about TDD at all, let’s just examine the diagram above. Why would anyone think that updating the tests because they are failing after a refactor is bad when we have “proof” like this out there? It basically says that after a refactor — tests fail. We fix them, then we end up with green tests. The problem is that this image lacks the process between the steps, not like this one below:

Well, this one actually tells us the truth that after a refactor you aren’t going to end up with a failing test. On the contrary, you need them to pass to advance.

Definition of a unit

What is a unit?

Intuitively, one can view a unit as the smallest testable part of an application. — Wikipedia

Intuitively. This is the word where everything has gone wrong. If anyone speaks or writes about unit testing he means testing a method, testing a class, a function.

Isolating everything else, examining the internals of that particular code. If we use style checking, Sonar, etc. then we will end up with small methods, low complexity, short classes that do one thing. But does this class with a few lines of code makes sense for the business? Does an if statement represent a functionality? Let’s look at how most of the developers write unit tests!

Our usual unit tests

In an OOP environment we used to create a test class for a production class. Testing all the public methods of that class and by replacing every collaborator with test doubles to help focus on the logic inside.

Let’s say we have classes like this:

• UserService
• EmailValidator
• UserRepository

Then it’s perfectly safe to assume that we have tests like this:

• UserServiceTest
• EmailValidatorTest
• UserRepositoryTest

Is this a problem? Not until we change anything. We learned the importance of loose coupling long before but we still fail to apply it to our tests.

With this approach we glue the structure of our tests to our production classes which makes refactoring even harder. Even though they both describe the same behavior, their structure may differ.

I think — and you might not accept it — that a class is not the smallest testable unit. Most of the time multiple class are created for the simplest task. In my opinion that small group of classes could be isolated and could be called a unit.

Let’s look at an example! We will be using Java as it’s one of the most popular language around. It will be dumb enough to understand but still complex enough to be able to display the problem.

We have a simple service, which is able to create users. There is a request coming from somewhere containing the information needed to create it. We perform basic validations — null checks to be precise — a simple transformation and then we pass it towards some persistence layer.

Let’s look at how we structure our tests. We will follow the one test — one class rule at the beginning.

We create a mock for the repository and pass it over through a constructor to our service.

And then we create a few test cases for it. One for the validation, one for the transformation and one to check if it persists the transformed user.

We are checking small parts of the code. We should not verify the whole method over and over again because in that case our tests coverage would highly overlap and it means our tests are not specific enough to pinpoint where the failure is.

So we have our tests and when we run them they are green. That’s our starting point.

Let’s start to refactoring our code. First we just extract the logic to private methods:

It may look like overkill with these one liners but bear with me. This technique applies to bigger code bases as well.

Note: It’s obvious that our private methods are being tested through the public method.

We did our changes to the production code. We rerun our tests and they are still green but this isn’t news.

Let’s move on with our refactor. How about extracting the validator to a new class?

We create a new class for it with a single method containing that logic from our service:

And we add this to our UserService class as a new collaborator and replace the previous private validate method.

So we done with our refactor and even if we aren’t following TDD, our next step will be to rerun all the tests and expect them to pass.

But they won’t.

It fails with a compile error because we’ve added a new constructor parameter and our test doesn’t recognize it yet. But who cares, right? Let’s just create a test double for that and pass it over:

But if you create a test double then you need to set it up correctly because otherwise it won’t throw any exception that is failing your test. It might seem a small change but that’s because our example is small too. And we still had to touch our tests even though the behavior of the create method hasn’t changed. And this is the moment when we

lost the safety our tests should have given us.

But nothing is lost, let’s revert our changes and go back to the step when we extracted our logic into private methods. Tests are green so we can start refactoring again. First step, we create a factory for our service:

We name the method createDefault. The naming isn’t new at all, there are bunch of libraries that offer default implementations via a factory or a builder. With those they are able to use the library without knowing how to orchestrate all the classes to make things work. Our method could be static too because we won’t mock it out, there are no test unfriendly features in it, like I/O or threads and we could use it with our IoC container as well.

Let’s replace the constructor in our setup method:

With this we introduced a new layer between the test and our constructor, and eliminated the interface coupling of the constructor. We don’t see any benefits yet, but let’s rerun the tests. They are green.

Now there’s nothing left but to repeat our last step: extract the validation logic to a new class — which caused a compile error before — and update this factory, which is still production code, right? So we are allowed to do that:

As we are speaking of a default implementation we just pass in a new validator. We could obviously overload this method for our IoC container.

If you are a strict mockist person, then you now might look like this:

What have I done? Why am I using the original implementation instead of a mock? Because it’s part of the test. We moved that logic out to a new class but we are still testing that part. Instead of creating a brand new UserCreateRequestValidatorTest, we are doing it in the UserServiceTest. So if we look at the coverage here, we could see that all branches are covered:

So what did we do? We are using a slightly different mindset than before and we let our tests cover a bigger scope. We let it to extend beyond our UserService to leak it into its collaborators. As we test a larger scope we might split our tests a bit differently. So one unit test will focus on a functionality of a single method in a class. We create new tests for the other methods if needed but they will still use the collaborators.

What kind of collaborators could we use here? The ones which have a deterministic output. Pure functions which always return the same for the same input. When a null parameter is passed towards our validator it will throw an exception no matter what. These are like Math functions. If you call Math.floor(5.1) there is no chance that somehow it returns anything but 5.

But we still need to use test doubles for the collaborators which doesn’t have a deterministic output, like our repository.

Let’s move on with our refactor!

We extract the transformer as well:

Update the UserService to use it:

And add it to the factory:

Rerun our tests: still green.

What about classes reused across units? Are these tested over and over? With the same scenarios? Indeed. But these scenarios are tested in a different context. So if we have a collaborator used in two units and we refactor this collaborator and end up with one of the unit tests failing we will know that we made a change which is not applicable to that context.

Side note on using mocks

What could go wrong if we used mocks instead of the real collaborators? Let’s get back to the step when we used a mock validator:

In this case we setup our mock to throw an exception when a null parameter is given. We even have to tell it what kind of exception it should throw.

But what if in the meanwhile somebody updated that validator? Do we trust that they also updated this test? What if this method now simply logs a message if a null parameter arrives instead of throwing an exception? They might have updated the corresponding test called XYValidatorTest, but here we are testing a scenario which should not happen. Of course a higher level test could catch these errors, but looking at the test pyramids we know that we create less higher level tests and maybe this bug ends up in production. The bigger problem is that it will be quite hard to find it because we messed up our best documentation, our test. It lies to us the same way comments do.

In summary:

• we could reduce the time of setting up mocks
• decouple our constructor from our test
• use the original implementations where it applies

and all of these will help us to create an environment where safe refactoring isn’t just a dream anymore.

The dummy, the spy and the ten little stubs

TL;DR

Dummy: placeholder. Stub: preloading methods with data. Spy: stub with the ability to monitor the calls. Mock: spy which verifies itself. Fake: a faster working implementation.

Do you know whats the difference between a mock and a stub? What is being created when you invoke your mocking framework’s createMock method? Does it spit a mock out? Are you sure? Believe me, most of the developers will fail to answer these types of questions (and not only due to the fact that they don’t even have tests on their project, let’s ignore this sad fact for now). The reason is that most people follow habits. These habits are based on the tests already in the codebase. „We have to stick to the conventions” they say. „Verifying everything is not written down anywhere but that’s how things used to be”.

People are afraid of creating bugs. Unfortunately, that’s what we do most of the time even if we are not aware of this. You don’t want to fix bugs. You don’t want to show up on your teams’ wall of shame. Hell, there are companies out there that even reward if you maintain your bug counter below a certain threshold.

Everything is about the numbers.

So you end up verifying everything in every method just to be 100% sure it works the same way. The problem with this is they think of this ‘same way’ like the same implementation and not with the same state. Another problem is that verifying everything every time repeatedly in your tests won’t stop bugs appearing:

Because thats not what tests are for. Thats only their side effect.

Okay, let’s get back to the original topic and start with the basics. Let’s speak about the test doubles themselves. There are a few types, by name: dummy, stub, mock, spy and fake.

Dummy: these are what their name suggests. Placeholders. When you need a parameter somewhere with a specific type, then you create a dummy. Depending on how hard is to instantiate it you could use an actual instance of the type or use your mocking framework to do that. Even a null might be enough. The thumb rule is that if its hard to instantiate it then use the framework. Sometimes we even add checks to this because we create and pass them for a simple purpose. To be placeholders. So when a method got invoked on them we might want to throw an exception.

class DummyOrder extends Order {
  @Override
  void applyDiscount(){
    throw new RuntimeException();
  }
}

That’s an extended version of the original parameter with an overridden method to make sure it is not being invoked.

To answer our question above: when you invoke the createMock method, first you got yourself a dummy. No behavior. No data in it. A plain object with dumb, null returning or void methods.

Stub: Okay, so here is our collaborator, let’s say we have factory and we invoke its create method. But as we said before at the dummy, every method of this object return nulls or primitives. But we need an actual object from it because otherwise we end up with a nullpointer exception on the next line. A moment later your colleague appears with a macchiato and some cool advice like “Man, use the expect method of EasyMock.. but don’t forget to verify it at the end of your test method otherwise you can’t be sure it got invoked and… “. Okay, we’ve had enough, let’s just ignore the second part. So a stub is nothing but a dummy which has some of their methods preloaded with data. We could state that “when this method is being invoked, return this object I gave you”. If it’s a simple factory you could even subclass it in your test (in Java at least) and hardcode a single value to be returned. Why do we even care about these return values? Because we want our tests to be deterministic. So any time we run the test and it invokes the previously stubbed method, it will return the same value over and over. And you could ask “Why would I do this?”. Well because if your inputs change but your outputs and expectations don’t, then you’ll end up with fragile, environment-dependent tests. Let’s say you want to test subscriptions and they work fine until 2018 but then it fails over and over. And thats because you used a real local DateTime provider without stubbing and after 2018 the current time is outside the threshold of the subscription start and end date. I wouldn’t be in your shoes when someone finds your name on that change. (Obviously, you could use expectations relative to the input to avoid this but that’s just more hassle).

class FixLocalDateTimeProvider extends LocalDateTimeProvider {
  @Override
  void get(){
    return LocalDateTime.of(2018,3,30,10,10,10);
  }
} 

Above is a very simple stub implementation which always returns the same date.

Mock: We arrived to the tough ones. Again start with a dummy. Do you remember what your colleague said before? That you can not be sure if this method actually got invoked? He just defined the purpose of a mock. So this purpose is to verify the method calls and in extreme situations the order of the calls. When do we verify the calls? If we need behaviour verification. And when do we do that? When we can’t do state verification.

Let’s take a little detour now and talk about these verifications. So state verification is when we can verify the state of the objects and we have certain expectations. For example, if you invoke a setter, you could verify the actual invocation via a mock (which is behaviour verification) but you could also call the getter and inspect what it returned to check its state. Of course, you should never test the behavior of setters directly but the point is that most of the time state verification is enough. But then when should we use mocks? Let’s imagine a void method. For example an asynchronous message publisher. We need to make sure that the message has been passed to it. We could subclass it and implement some sort of counter and then see if it got incremented (which will be a spy) but if we need to check its parameters as well it would be convenient to use a mock instead and verify that call along with the arguments.

class MockOrder extends Order {
  private boolean discountApplied = false;

  @Override
  void applyDiscount(){
    discountApplied = true;
  }

  boolean discountApplied(){
    return discountApplied;
  }

  void verifyDiscount(){
    assertTrue(discountApplied());
  }

}

Above is an implementation of a very simple mock (the name could be misleading as we are not verifying directly any method call here). In our tests, we can pass this to the system under test and later on call the verify method on it to check if the discount was applied by verifying its internal state.

And we arrived at the end of our detour: mockist and classicist groups. The first one prefers the usage of mocks and behaviour verification while the other aims to verify the state. When you create a mock in a test and expect some method calls, you bind that implementation to the test. Therefore if someone changes the implementation there is a higher chance that this test will break even though it still does its job, so the state of the objects is the same. Meanwhile verifying the state could demand methods to help it. You have to ask yourself: is it convincing me that the end result is correct, or am I more interested in how the method achieved it?

Spy: Let’s start with theoretical concept because many frameworks implement spies in a different way.

We are interested in whether a particular method got invoked, or maybe the parameters which it got during the test. We can create a spy which has some ‘interrogation’ method(s) on it. With the usage of this method(s), we could inspect if the spy being used the right way. Spies are like mocks but they are not verifying themselves.

class SpyOrder extends Order {
  private boolean discountApplied = false;

  @Override
  void applyDiscout(){
    discountApplied = true;
  }

  boolean discountApplied(){
    return discountApplied;
  }
}

At the example above we almost the same order class which we used at the mocks but this one doesn’t have a verify method. We have to manually check it. If we don’t want to create several verify methods for the possible states in the class its convenient to use spies instead. You still could combine these verifications in your tests.

Important note on spies: Some frameworks (like Mockito) use the term spy for proxies on actual objects which proxies add stubbing and mocking behaviour.

Fake: these doubles have some working implementation but much simpler therefore they cannot be used in production. There are some typical cases for this like in-memory databases, fake JSON APIs and so on. The purpose of these is to make tests faster.

class FakeUserRepository implements UserRepository {
  
  private HashMap<Long, User> map = new HashMap<>();

  void add(User user){
    map.put(user.getId(), user);
  }

  boolean findById(Long userId){
    return map.get(userId);
  }
}

Above is a fake repository implementation backing data in a simple hashmap instead of a real database. It is important that these kind of fake repositories aren’t for integration testing.

Let’s get back to our original topic. When you invoke that so-called createMock method, it spits out a dummy. When you configure it to return a specific value then it will become a stub. Let’s say you want to inspect the methods called: you got yourself a spy. If you also verify this behaviour then it turns to a mock. The fake is a totally different story which can’t be created by your mocking framework (or not that I am aware of) so you have to implement it by hand.

In case you verify every call in your tests (just as our imaginary colleague recommended) then you are creating only mocks and therefore bind the implementation to the tests. It will make your life quite hard at the time of refactoring so please don’t.

I hope this helped in better understanding of the test doubles and what they are for.

We recently had a podcast episode about dependencies and which ones to decouple from your domain along with how to do it. And here came the adapter pattern as a perfect solution to pair two different interfaces by internally pairing their contract. We used email sending as an example so I will use it in our example too. The main advantage mentioned was that we don’t need to know which implementation will be used later on so we could efficiently decouple our logic from it.

Okay, but how?

We will start from the business/domain point of view as it should happen every time. The statement was that we might only know the email address, so let’s create a mailer interface and later on we could create a fake implementation for it. When we find the right candidate for implementing this email sending, we will create an adapter between them. Simple as that. This interface will act as a boundary or as a gateway towards the mail sending in terms of the architecture.

interface MailSender {
}

Okay, we got our interface, what should we put in there? Well, obviously we will have a send method:

interface MailSender {

    public function send($from, $to, $subject, $content);

}

If we got these parameters, our PHPMailer implementation will be quite dumb. Taking parameters, mixing them in a way, and passing it to PHPMailer:

class PhpMailSenderAdapter implements MailSender {

    /**
     * @var PHPMailer
     */
    private $phpMailer;

    public function __construct(PHPMailer $phpMailer)
    {
        $this->phpMailer = $phpMailer;
    }

    public function send($from, $to, $subject, $content)
    {
        $this->phpMailer->setFrom($from);
        $this->phpMailer->addAddress($to);
        $this->phpMailer->isHTML(true);
        $this->phpMailer->Subject = $subject;
        $this->phpMailer->Body    = $content;
        $this->phpMailer->send();
    }
}

But as I said, we are approaching this problem from the business point of view and we are only sending one type of e-mail, let’s say weekly newsletters. Then we shouldn’t have that generic solution, right? We also want to ignore anything related to the actual mail sending as long as we can while focusing on the core domain just as Uncle Bob recommends in his books. Let’s look at where we end up if we create the service for this specific task:

interface MailSender {

    public function sendNewsletter($to, $content);

}

We pass in a minimal content: the address we are sending these newsletter stuff to and the actual content of the email. Therefore we have to gather this in the adapter somehow or else we end up with hard coded stuff like this:

public function sendNewsletter($to, $content)
{
    $this->phpMailer->setFrom("noreply@letscode.hu");
    $this->phpMailer->addAddress($to);
    $this->phpMailer->isHTML(true);
    $this->phpMailer->Subject = "Weekly newsletter";
    $this->phpMailer->Body    = $content;
    $this->phpMailer->send();
}

Okay, let’s assume that we were able to manage the above and pull together some services which provides us these previously hard coded details, or we just inject them via DI:

public function sendNewsletter($to, $content)
{
    $this->phpMailer->setFrom($fromAddress);
    $this->phpMailer->addAddress($to);
    $this->phpMailer->isHTML(true);
    $this->phpMailer->Subject = $newsletterSubject;
    $this->phpMailer->Body    = $content;
    $this->phpMailer->send();
}

Nice one, right? We could do the same for SwiftMailer in no time, huh?

NEVER.DO.THAT.

(And not because we are just adding the addressess to a single PHPMailer instance, let’s ignore that fact for a second since this is just a simple example.)

Okay, so what’s the problem with the non-generic solution above? At the end of the day it does the job, maps one interface to another, right? Then why should we stick to the first generic solution and move logic from the non-generic one?

There are a couple of reasons but first of all this clearly violates the SRP as we are not just mapping these interfaces but also gathering information. If the PHPMailer interface (or whatever we are about to use) changes, then we have to update this code.

Let’s say we are about to introduce localisation or anything changes regarding the business, then we have to add it here as well. An adapter is an adapter, it should NEVER contain anything business related. Its client should provide every necessary information. Excluding the infrastructure related things like the SMTP host/port, the username and password in this case. We also missed the isHTML method which still could be business related as we might want to send plain text mails like Jenkins alerts or whatever.

In this example the actual mail sending is not part of the domain, it’s part of the infrastructure. Therefore the adapter’s job is to form a boundary between the perimeter of your domain layer towards the perimeter of the infrastructure layer. If you put domain logic in there, then you disperse your logic through layers which is a bad thing as you should keep these in one place.

Let’s look at what happens when you decide to use Swiftmailer instead. Then you will introduce this aforementioned logic in that so called “adapter” as well. If that logic changes, you have to update both of these classes although none of the interfaces changed. Obviously we could create an abstraction layer and wrap the adapter while feeding the necessary information, which sounds quite reasonable, right?

interface MailInterface {

    public function send($from, $to, $subject, $content);

}

class NewsletterService {

    /**
     * @var MailInterface
     */
    private $mailInterface;


    public function sendNewsletter($to) {
        $this->mailInterface->send("noreply@letscode.hu", $to, "Weekly newsletter", "somecontent");
    }

}

In the above example we stick to the generic solution for the adapter and wrap it in the NewsletterService. The NewsletterService will be responsible for gathering the necessary information and the MailInterface will act as an adapter towards whatever mailer implementation we are using.

Win-win.

Update: When we are implementing a more generic solution for notifying our users like Twitter/e-mail/SMS then we should create another layer of abstraction above the aforementioned solution. SMS won’t have subject nor tweets but the e-mails does.

First of all let’s see the definition: what counts as legacy code? The word ‘legacy’ isn’t the synonym for ‘crap’, nor for ‘endless pain’ and there isn’t any evidence that it must be COBOL or FORTRAN either.

If we ask wikipedia about this it will tell us a bit more about the topic. It states that a codebase becomes legacy if it is related to non-supported technologies, e.g. we must support serial port, windows XP, IE7 or PHP 5.2.4. So Wikipedia generally says that backward compatibility is the main issue mostly if we maintain code for waaaaaaaay too long. If we think about Wordpress, we already got the first case where they can’t clean up because it still compatible with PHP 5.2.4!

Most of the time we don’t have to travel back several years in time to find such monsters. There are several ways to work as a programmer. If we highlight PHP then we could be employed by a multinational company or a smaller one or we could do it as a freelancer. Working at a big company doesn’t means that people actually work together as there are several companies where they just churning out microsites. Most of the employers are hiring ‘full-stack gurus’ nowadays and the developers there can be assigned to a single project which they could finish without ever speaking to others than the designer. This also applies to freelancers and smaller companies. And when someone works alone for a longer period of time they get lazy. No one looks at their code, no one blames them, and there are no standards to comply to. But there is one thing:

the deadline.

If there aren’t any quality expectations then there will be quantity expectations and those will come from the management side.

Let’s look at the story of Dave:

Our full stack developer friend just got hired to a small company where they mostly pour out micro sites. They aren’t doing agile but instead worship something called ASAP Driven Development which basically means that everything should be done the day before. Dave is a developer with principles so he tries to organize his code well, using translation keys in the view files, etc.. by and large he does a great job. With this attitude he could ship smaller sites in a reasonable pace. But the project manager has a motto which states “you should drop more and more tasks on the developer because as long as he can carry it he has more capacity” thus he find our Dave’s work slow and expects him to do better.

On one of those status meetings Dave accidentally mentions “if I hurry even more the code would be ugly”. The PM needs no more confirmation so he rebukes poor Dave as he’s wasting his time on non-business related factors is slower than the others. As software developers are not known for their assertive behaviour, Dave couldn’t stand up for himself so he falls in line with the other coders. From now on he refuses to write unit tests, ignores SOLID principles, mindlessly copies classes from the previous projects, hacks into composer packages because there is no time for the pull requests and so on. The project manager is glad as he noticed that Dave’s performance was twenty percent better than ever.

As the months pass, Dave attempts to be even more productive. He queries the database in the view files, drops frameworks and carries a functions.dave.php file which gain weight from project to project. As he became the most productive developer, management decides to promote him. He becomes a senior developer. The new title comes with new responsibilities, so from now on he has to ramp up newcomers as well. He makes them sit down at his computer, opens one of his past projects in the IDE and shows them the functions in his mighty functions.dave.php The poor guys learn the meaning of legacy code here.

But Dave wants a change in environment so he moves toward another company. This firm isn’t specialized on small sites but bigger, more complex web applications, where a product and their components are developed for years. A colleague described the ticket workflow to Dave:

• You develop the code. The code that has no tests against it is as if there were no code.
• If you are done with this then you ask a QA to help you out and together you investigate the issue, to see if you covered every necessary flow.
• Then you make a pull requests and ask us to review it. You also have to review others’ code!
• When you fixed the review items or discussed them and they approved your pull request THEN and ONLY THEN you should merge it to the master branch.
• You should spend 15% of your time on tech debts each sprint if possible.

This shocked Dave as it was a completely different approach than the one they followed in their previous job. The codebase was huge, hundreds of classes piled up, but every public interface or class was nicely documented and there were no two differently formatted files. During the reviews he saw many new techniques while investigating other one devs’ code. They also held a weekly coding dojo where they could’ve learned every aspect of the IDE and the core API.

The promotion were not based on the amount of resolved tickets, quantity does not make up for quality here as the team must maintain the codebase and as the tech dept list grows it will hurt more. The managers were aware of that fact. This was a different kind of legacy code which is not bad, just old, but due to the continous refactoring there were no part of it which hasn’t been revamped a bit.

But let’s see another example.

We have a codebase, which had been written in prehistoric PHP more or less 10 years ago. It was the time when PHP 5.2 got the JSON extension, but the hosting providers back then were like they are now, applying new versions pretty slowly.

Then we met this code and we could hardly hold back our tears because we see a function which builds up a JSON string ‘by hand’. What the ???

Then we check the docblock at the top of the file:

Date: 2006.02.11

After some quick calculations around PHP releases and we find that only the latest 5.2 version had json_encode/decode (Don’t forget PHP 4 was a thing then!). Then we think about that there are many hosting providers around which still runs PHP 5.5 so the whole code makes sense now.

But if that codebase were made last year we would question that person’s PHP knowledge. The developers of PHP are working hard to extend the available toolset and hence make our work more efficient so don’t be surprised if there was no Java 8 calls 10 years before or JSON encode while using .NET 2.0.

The PHP is a quite special case as the ratio of freelancers are higher and they tend to work alone. As I have mentioned before, if someone works alone then they tend to be lazy, negligent as there is no force making them comply to standards, principles.

What could we do to avoid leaving monsters like this behind?

• If your company doesn’t respect code quality then leave it or take refactoring into account while making estimations and don’t let others influence you in this. Principles!
• Work together. If you ended up at the same place, learn from each other. Other people may know some fancy keyboard shortcuts or API call which you have no idea about.
• If you aren’t able to work together during office hours then attend Coding Dojos or organize one in the office.
• Open-source! If you are working alone then hop on a github project and you’ll be able to cooperate and learn from others.

The moral of the story: A codebase don’t have to be old in order to make you cry.