104

I'm attempting to get into the habit of writing unit tests regularly with my code, but I've read that first it's important to write testable code. This question touches on SOLID principles of writing testable code, but I want to know if those design principles are beneficial (or at least not harmful) without planning on writing tests at all. To clarify - I understand the importance of writing tests; this is not a question on their usefulness.

To illustrate my confusion, in the piece that inspired this question, the writer gives an example of a function that checks the current time, and returns some value depending on the time. The author points to this as bad code because it produces the data (the time) it uses internally, thus making it difficult to test. To me, though, it seems like overkill to pass in the time as an argument. At some point the value needs to be initialized, and why not closest to consumption? Plus, the purpose of the method in my mind is to return some value based on the current time, by making it a parameter you imply that this purpose can/should be changed. This, and other questions, lead me to wonder if testable code was synonymous with "better" code.

Is writing testable code still good practice even in the absence of tests?


Is testable code actually more stable? has been suggested as a duplicate. However, that question is about the "stability" of the code, but I am asking more broadly about whether the code is superior for other reasons as well, such as readability, performance, coupling, and so forth.

  • 24
    There's a special property of the function that requires you to pass in the time called idempotency. Such a function will produce the same result each time it is called with a given argument value, which not only makes it more testable, but more composable and easier to reason about. – Robert Harvey Jul 1 '15 at 15:22
  • 4
    Can you define "better code"? do you mean "maintainable"?, "easier-toUse-without-IOC-Container-Magic"? – k3b Jul 1 '15 at 16:11
  • 7
    I guess you've never had a test fail because it used the actual system time and then the time zone offset changed. – Andy Jul 1 '15 at 16:34
  • 5
    It is better than untestable code. – Tulains Córdova Jul 1 '15 at 20:17
  • 14
    @RobertHarvey I wouldn't call that idempotency, I would say it's referential transparency: if func(X) returns "Morning", then replacing all occurences of func(X) with "Morning" will not change the program (ie. calling func doesn't do anything other than return the value). Idempotency implies either that func(func(X)) == X (which is not type-correct), or that func(X); func(X); performs the same side-effects as func(X) (but there are no side-effects here) – Warbo Jul 1 '15 at 21:18

12 Answers 12

117

In regard to the common definition of unit tests, I'd say no. I've seen simple code made convoluted because of the need to twist it to suit the testing framework (eg. interfaces and IoC everywhere making things difficult to follow through layers of interface calls and data that should be obvious passed in by magic). Given the choice between code that is easy to understand or code that is easy to unit test, I go with the maintainable code every time.

This doesn't mean not to test, but to fit the tools to suit you, not the other way round. There are other ways to test (but difficult-to-understand code is always bad code). For example, you can create unit tests that are less granular (eg. Martin Fowler's attitude that a unit is generally a class, not a method), or you can hit your program with automated integration tests instead. Such may not be as pretty as your testing framework lights up with green ticks, but we're after tested code, not the gamification of the process, right?

You can make your code easy to maintain and still be good for unit tests by defining good interfaces between them and then writing tests that exercise the public interface of the component; or you could get a better test framework (one that replaces functions at runtime to mock them, rather than requiring the code to be compiled with mocks in place). A better unit test framework lets you replace the system GetCurrentTime() functionality with your own, at runtime, so you don't need to introduce artificial wrappers to this just to suit the test tool.

  • 3
    Comments are not for extended discussion; this conversation has been moved to chat. – World Engineer Jul 2 '15 at 18:44
  • 2
    I think it's worth noting that I know of at least one language that allows you to do what your last paragraph describes: Python with Mock. Because of the way module imports work, pretty much anything aside from keywords can be replaced with a mock, even standard API methods/classes/etc. So that's possible, but it might require that the language be architected in a way the supports that kind of flexibility. – jpmc26 Jul 4 '15 at 6:26
  • 6
    I think there's a difference between "testable code" and "code [twisted] to suit the testing framework". I'm not sure where I'm going with this comment, other than to say I agree that "twisted" code is bad, and "testable" code with good interfaces is good. – Bryan Oakley Jul 5 '15 at 22:28
  • 2
    I expressed some of my thoughts in the article's comments (since extended comments are not allowed here), check it out! To be clear: I am the author of the mentioned article :) – Sergey Kolodiy Jul 6 '15 at 9:16
  • I gotta agree with @BryanOakley. "Testable code" suggest your concerns are separated: it's possible to test a aspect (module) without interference from other aspects. I would say this is different from "adjusting your project support specific testing conventions". This is similar to design patterns: they shouldn't be forced. Code that properly utilizes design patterns would be considered strong code. Same applies for testing principles. If making your code "testable" results in twisting your project's code excessively, you're doing something wrong. – Vince Emigh Jun 6 '16 at 19:13
69

Is writing testable code still good practice even in the absence of tests?

First things first, an absence of tests is a way bigger issue than your code being testable or not. Not having unit tests means you're not done with your code/feature.

That out of the way, I wouldn't say that it's important to write testable code - it's important to write flexible code. Inflexible code is hard to test, so there's a lot of overlap there in approach and what people call it.

So to me, there is always a set of priorities in writing code:

  1. Make it work - if the code doesn't do what it needs to do, it is worthless.
  2. Make it maintainable - if the code isn't maintainable, it will quickly stop working.
  3. Make it flexible - if the code isn't flexible, it will stop working when business inevitably comes by and asks if the code can do XYZ.
  4. Make it fast - beyond a base acceptable level, performance is just gravy.

Unit tests help code maintainability, but only to a point. If you make the code less readable, or more fragile to make the unit tests work, that gets counter productive. "Testable code" is generally flexible code, so that is good, but not as important as functionality or maintainability. For something like the current time, making that flexible is good but it harms maintainability by making the code harder to use right and more complex. Since maintainability is more important, I will usually err towards the simpler approach even if it is less testable.

  • 4
    I like the relationship you point out between testable and flexible - that makes the whole problem more understandable to me. Flexibility allows your code to adapt, but necessarily makes it a bit more abstract and less intuitive to understand, but that's a worthwhile sacrifice for the benefits. – WannabeCoder Jul 1 '15 at 15:19
  • 3
    that said, often I see methods that should have been private being forced public or package level in order for the unit testing framework to be able to access them directly. Far from an ideal approach. – jwenting Jul 2 '15 at 5:37
  • 4
    @WannabeCoder Of course, it's only worth adding flexibility when it saves you time in the end. That's why we don't write every single method against an interface - most of the time it's just easier to rewrite the code rather than incorporating too much flexibility from the onset. YAGNI is still an extremely powerful principle - just make sure that whatever the thing "you're not going to need" is, adding it retroactively will not give you more work on average than implementing it ahead of time. It's the code not following YAGNI that has the most issues with flexibility in my experience. – Luaan Jul 2 '15 at 7:45
  • 3
    "Not having unit tests means you're not done with your code/feature" - Untrue. The "definition of done" is something that the team decides. It may or may not include some degree of test coverage. But nowhere is there a strict requirement that says a feature cannot be "done" if there are no tests for it. The team may choose to require tests, or they may not. – aroth Jul 3 '15 at 11:22
  • 3
    @Telastyn In over 10 years of development, I never had a team that mandated a unit testing framework, and only two that even had one (both had poor coverage). One place required a word document of how to test the feature you were writing. That's it. Perhaps I'm unlucky? I'm not anti-unit test (seriously, I mod the SQA.SE site, I'm very pro unit test!) but I haven't found them to be as widespread as your statement claims. – corsiKa Jul 3 '15 at 16:54
51

Yes, it is good practice. The reason is that testability is not for the sake of tests. It is for the sake of clarity and understandability that it brings with it.

Nobody cares about the tests themselves. It is a sad fact of life that we need large regression test suites because we're not brilliant enough to write perfect code without constantly checking our footing. If we could, the concept of tests would be unknown, and all of this wouldn't be an issue. I certainly wish I could. But experience has shown that almost all of us can't, therefore tests covering our code are a good thing even if they take away time from writing business code.

How does having tests improve our business code independently of the test themselves? By forcing us to segment our functionality into units that are easily demonstrated to be correct. These units are also easier to get right than the ones we'd otherwise be tempted to write.

Your time example is a good point. As long as you only have a function returning the current time you might think that there's no point in having it programmable. How hard can it be to get this right? But inevitably your program will use this function in other code, and you definitely want to test that code under different conditions, including at different times. Therefore it's a good idea to be able to manipulate the time that your function returns - not because you mistrust your one-line currentMillis() call, but because you need to verify the callers of that call under controlled circumstances. So you see, having code testable is useful even if on its own, it doesn't seem to merit that much attention.

  • Another example is if you want to pull out some part of the code of one project to some other place (for whatever reason). The more the different parts of functionality are independent of each other the easier it is to extract exactly the functionality you need and nothing more. – valenterry Jul 1 '15 at 15:14
  • 10
    Nobody cares about the tests themselves -- I do. I find tests to be a better documentation of what the code does than any comments or readme files. – jcollum Jul 1 '15 at 17:59
  • I've been slowly reading about testing practices for a while now (as somehow who does no unit testing at all yet) and I have to say, the last part about verifying the call under controlled circumstances, and the more flexible code that comes with it, made all sorts of things click into place. Thank you. – plast1k Jul 2 '15 at 3:54
12

At some point the value needs to be initialized, and why not closest to consumption?

Because you may need to reuse that code, with a different value than the one generated internally. The ability to insert the value you are going to use as a parameter, ensures that you can generate those values based on any time you like, not just "now" (with "now" meaning when you call the code).

Making code testable in effect means making code that can (from the start) be used in two different scenarios (production and test).

Basically, while you can argue there is no incentive to make the code testable in the absence of tests, there is great advantage in writing reusable code, and the two are synonyms.

Plus, the purpose of the method in my mind is to return some value based on the current time, by making it a parameter you imply that this purpose can/should be changed.

You could also argue that the purpose of this method is to return some value based on a time value, and you need it to generate that based on "now". One of them is more flexible, and if you get used to choosing that variant, in time, your rate of code reuse will go up.

10

It may seem silly to say it this way, but if you want to be able to test your code, then yes, writing testable code is better. You ask:

At some point the value needs to be initialized, and why not closest to consumption?

Precisely because, in the example you are referring to, it makes that code untestable. Unless you only run a subset of your tests at different times of the day. Or you reset the system clock. Or some other workaround. All of which are worse than simply making your code flexible.

In addition to being inflexible, that small method in question has two responsibilities: (1) getting the system time and then (2) returning some value based on it.

public static string GetTimeOfDay()
{
    DateTime time = DateTime.Now;
    if (time.Hour >= 0 && time.Hour < 6)
    {
        return "Night";
    }
    if (time.Hour >= 6 && time.Hour < 12)
    {
        return "Morning";
    }
    if (time.Hour >= 12 && time.Hour < 18)
    {
        return "Afternoon";
    }
    return "Evening";
}

It makes sense to break down the responsibilities further, so that the part out of your control (DateTime.Now) has the least impact on the rest of your code. Doing so will make the code above simpler, with the side effect of being systematically testable.

  • 1
    So you'd have to test early in the morning to check that you get a result of "Night" when you want it. That's difficult. Now assume you want to check that date handling is correct on Feb 29th 2016... And some iOS programmers (and probably others) are plagued by a beginner's mistake that messes thing up shortly before or after the start of the year, how do you test for that. And from experience, I would check date handling on Feb. 2nd, 2020. – gnasher729 Jul 1 '15 at 21:30
  • 1
    @gnasher729 Exactly my point. "Making this code testable" is a simple change that can solve a lot of (testing) problems. If you don't want to automate testing, then I guess the code is passable as-is. But it would be better once it's "testable". – Eric King Jul 1 '15 at 21:53
9

It certainly has a cost, but some developers are so accustomed to paying it that they've forgotten the cost is there. For your example, you now have two units instead of one, you've required the calling code to initialize and manage an additional dependency, and while GetTimeOfDay is more testable, you are right back in the same boat testing your new IDateTimeProvider. It's just that if you have good tests, the benefits usually outweigh the costs.

Also, to a certain degree, writing testable code sort of encourages you to architect your code in a more maintainable way. The new dependency management code is annoying, so you'll want to group all your time-dependent functions together, if at all possible. That can help to mitigate and fix bugs like, for example, when you load a page right on a time boundary, having some elements rendered using the before time and some using the after time. It can also speed up your program by avoiding repeated system calls to get the current time.

Of course, those architectural improvements are highly dependent on someone noticing the opportunities and implementing them. One of the biggest dangers of focusing so closely on units is losing sight of the bigger picture.

Many unit test frameworks let you monkey patch a mock object in at runtime, which lets you get the benefits of testability without all the mess. I've even seen it done in C++. Look into that ability in situations where it looks like the testability cost isn't worth it.

  • +1 - you do need to improve design and architecture to make writing unit tests easier. – BЈовић Jul 1 '15 at 16:21
  • 3
    + - its the architecture of your code that matters. Easier testing is just a happy side-effect. – gbjbaanb Jul 2 '15 at 9:21
8

It's possible that not every characteristic which contributes to testability is desirable outside the context of testability - I have trouble coming up with a non-test-related justification for the time parameter you cite, for instance - but broadly speaking the characteristics which contribute to testability also contribute to good code regardless of testability.

Broadly speaking, testable code is malleable code. It's in small, discrete, cohesive, chunks, so individual bits can be called for reuse. It's well-organized and well-named (to be able to test some functionality you give more attention to naming; if you weren't writing tests the name for a single-use function would matter less). It tends to be more parametric (like your time example), so open to use from other contexts than the original intended purpose. It's DRY, so less cluttered and easier to comprehend.

Yes. It's good practice to write testable code, even irrespective of testing.

  • disagree about it being DRY - wrapping GetCurrentTime in a method MyGetCurrentTime is very much repeating the OS call with no benefit except to assist the test tooling. That's just the simplest of examples, they get much worse in reality. – gbjbaanb Jul 2 '15 at 9:18
  • 1
    "repating the OS call with no benifit" -- untill you end up running on a server with one clock, speaking to an aws server in a different timezone, and that breaks your code, and you then have to go through all your code and update it to use the MyGetCurrentTime, which instead returns UTC. ; clock skew, daylight saving, and there are other reasons why it might not be a good idea to blindly trust the OS call, or atleast have a single point where you can drop in another replacement. – Andrew Hill Jul 4 '15 at 10:06
8

Writing testable code is important if you want to be able to prove that your code actually works.

I tend to agree with the negative sentiments about warping your code into heinous contortions just to fit it to a particular test framework.

On the other hand, everybody here has, at some point or other, had to deal with that 1,000 line long magic function that is just heinous to have to deal with, virtually can't be touched without breaking one or more obscure, non-obvious dependencies somewhere else (or somewhere within itself, where the dependency is nearly impossible to visualize) and is pretty much by definition untestable. The notion (which is not without merit) that testing frameworks have become overblown should not be taken as a free license to write poor quality, untestable code, in my opinion.

Test-driven development ideals do tend to push you toward writing single-responsibility procedures, for example, and that is definitely a good thing. Personally, I say buy into single-responsibility, single source of truth, controlled scope (no freakin' global variables) and keep brittle dependencies to a minimum, and your code will be testable. Testable by some specific testing framework? Who knows. But then maybe it's the testing framework that needs to adjust itself to good code, and not the other way around.

But just to be clear, code that is so clever, or so long and/or interdependent that it is not easily comprehended by another human being is not good code. And it also, coincidentally, isn't code that can easily be tested.

So nearing my summary, is testable code better code?

I don't know, maybe not. People here have some valid points.

But I do believe that better code tends to also be testable code.

And that if you're talking about serious software for use in serious endeavors, shipping untested code is not the most responsible thing you could be doing with your employer's or your customers' money.

It's also true that some code requires more rigorous testing than other code and it's a little silly to pretend otherwise. How would you like to have been an astronaut on the space shuttle if the menu system that interfaced you with the vital systems on the shuttle wasn't tested? Or an employee at a nuclear plant where the software systems monitoring temperature in the reactor weren't tested? On the other hand, does a bit of code generating a simple read-only report require a container-truck full of documentation and a thousand unit tests? I sure hope not. Just saying...

  • 1
    "better code tends to also be testable code" This is the key. Making it testable does not make it better. Making it better often makes it testable, and the tests often give you information you can use to make it better, but the mere presence of tests does not imply quality, and there are (rare) exceptions. – anaximander Jul 2 '15 at 10:30
  • 1
    Exactly. Consider the contrapositive. If it's untestable code, it's not tested. If it's not tested, how do you know whether it works or not other than in a live situation? – pjc50 Jul 2 '15 at 10:39
  • 1
    All testing proves is that the code passes the tests. Otherwise unit tested code would be bug free and we know that isn't the case. – wobbily_col Jul 2 '15 at 11:26
  • @anaximander Exactly. There is at least the possibility that the mere presence of tests is a contraindication which results in poorer quality code if all the focus is just on checking the checkboxes. "At least seven unit test for each function?" "Check." But I really do believe that if the code is quality code, it is going to be easier to test. – Craig Jul 2 '15 at 20:51
  • 1
    ...but making a bureaucracy out of testing can be a total waste and not produce useful information or trustworthy results. Regardless; I sure wish somebody had tested the SSL Heartbleed bug, yeah? or the Apple goto fail bug? – Craig Jul 2 '15 at 21:06
5

To me, though, it seems like overkill to pass in the time as an argument.

You're right, and with mocking you can make the code testable and avoid passing the time (pun intention undefined). Example code:

def time_of_day():
    return datetime.datetime.utcnow().strftime('%H:%M:%S')

Now let's say you want to test what happens during a leap second. As you say, to test this the overkill way you would have to change the (production) code:

def time_of_day(now=None):
    now = now if now is not None else datetime.datetime.utcnow()
    return now.strftime('%H:%M:%S')

If Python supported leap seconds the test code would look like this:

def test_handle_leap_second(self):
    actual = time_of_day(
        now=datetime.datetime(year=2015, month=6, day=30, hour=23, minute=59, second=60)
    expected = '23:59:60'
    self.assertEquals(actual, expected)

You can test this, but the code is more complex than necessary and the tests still can't reliably exercise the code branch that most production code will be using (that is, not passing a value for now). You work around this by using a mock. Starting from the original production code:

def time_of_day():
    return datetime.datetime.utcnow().strftime('%H:%M:%S')

Test code:

@unittest.patch('datetime.datetime.utcnow')
def test_handle_leap_second(self, utcnow_mock):
    utcnow_mock.return_value = datetime.datetime(
        year=2015, month=6, day=30, hour=23, minute=59, second=60)
    actual = time_of_day()
    expected = '23:59:60'
    self.assertEquals(actual, expected)

This gives several benefits:

  • You're testing time_of_day independently of its dependencies.
  • You're testing the same code path as production code.
  • The production code is as simple as possible.

On a side note, it is to be hoped that future mocking frameworks will make things like this easier. For example, since you have to refer to the mocked function as a string you can't easily make IDEs change it automatically when time_of_day starts using another source for time.

  • FYI: your default argument is wrong. It will only be defined once, so your function will always return the time it was first evaluated. – ahruss Jul 4 '15 at 17:41
4

A quality of well-written code is that it is robust to change. That is, when a requirements change comes along, the change in the code should be proportional. This is an ideal (and not always achieved), but writing testable code helps get us closer to this goal.

Why does it help get us closer? In production, our code operates within the production environment, including integrating and interacting with all of our other code. In unit tests, we sweep away much of this environment. Our code now is being robust to change because tests are a change. We are using the units in different ways, with different inputs (mocks, bad inputs that might never actually get passed in, etc) than we would use them in production.

This prepares our code for the day when change happens in our system. Let's say our time calculation needs to take a different time based on a timezone. Now we have the ability to pass in that time and not have to make any changes to the code. When we don't want to pass in a time and want to use the current time, we could just use a default argument. Our code is robust to change because it is testable.

4

From my experience, one of the most important and most far-reaching decisions you make when building a program is how you break the code down into units (where "units" is used in its broadest sense). If you are using a class-based OO language, you need to break all the internal mechanisms used to implement the program into some number of classes. Then you need to break each class' code into some number of methods. In some languages, the choice is how to break your code into functions. Or if you do the SOA thing, you need to decide how many services you will build and what will go into each service.

The breakdown which you choose has an enormous effect on the whole process. Good choices make the code easier to write, and result in fewer bugs (even before you start testing and debugging). They make it easier to change and maintain. Interestingly, it turns out that once you find a good breakdown, it is usually also easier to test than a poor one.

Why is this so? I don't think I can understand and explain all the reasons. But one reason is that a good breakdown invariably means choosing a moderate "grain size" for units of implementation. You don't want to cram too much functionality and too much logic into a single class/method/function/module/etc. This makes your code easier to read and easier to write, but it also makes it easier to test.

It's not just that, though. A good internal design means that the expected behavior (inputs/outputs/etc) of each unit of implementation can be defined clearly and precisely. This is important for testing. A good design usually means that each unit of implementation will have a moderate number of dependencies on the others. That makes your code easier for others to read and understand, but also makes it easier to test. The reasons go on; maybe others can articulate more reasons which I can't.

With regard to the example in your question, I don't think that "good code design" is equivalent to saying that all external dependencies (such as a dependency on the system clock) should always be "injected". That might be a good idea, but it is a separate issue from what I am describing here and I won't delve into its pros and cons.

Incidentally, even if you make direct calls to system functions which return the current time, act on the filesystem, and so on, this does not mean you can't unit-test your code in isolation. The trick is to use a special version of the standard libraries which allows you to fake the return values of system functions. I've never seen others mention this technique, but it is fairly simple to do with many languages and development platforms. (Hopefully your language runtime is open-source and easy to build. If executing your code involves a link step, hopefully it is also easy to control which libraries it is linked against.)

In summary, testable code is not necessarily "good" code, but "good" code is usually testable.

1

If you are going with SOLID principles you will be on the good side, especially if extend this with KISS, DRY, and YAGNI.

One missing point for me is the point of the complexity of a method. Is it a simple getter/setter method? Then just writing tests to satisfy your testing framework would be a waste of time.

If it's a more complex method where you manipulate data and want to be sure that it will work even if you have to change the internal logic, then it would be a great call for a test method. Many times I had to change a piece of code after several days/weeks/months, and I was really happy to have the test case. When first developing the method I tested it with the test method, and I was sure that it will work. After the change my primary test code still worked. So I was certain that my change didn't break some old code in production.

Another aspect of writing tests is to show other developers how to use your method. Many times a developer will search for an example on how to use a method and what the return value will be.

Just my two cents.

Not the answer you're looking for? Browse other questions tagged or ask your own question.