1

I want to write some invoicing logic, and I start coding it, using TDD.

The following example is silly, but I'm confident it represents well the everyday dilemma I'm facing

function createTestInvoice(client) { return Invoice.new( { client: client, rows: [ {item: 'apple', quantity: 1, unit_price_ 1} {item: 'banana', quantity: 2, unit_price: 2} ] } ) } al = Client.new({name: 'Al', can_buy_apples: true) assert( createTestInvoice(al).total ).to(equal(5)) john = Client.new({name: 'John', can_buy_apples: false) assert( createTestInvoice(al).total ).to(equal(4))

When I start with the implementation I realise that just the following

first naive implementation

is not enough, since I need some kind of InvoiceRowFactory that decides how and if to allow the row to be stored.

At this point the logic I was testing in this unit would be testing some logic that is included in the InvoiceRowFactory. A test for it would be:

/* remember: John cannot buy apples */ assert(InvoiceRowFactory.call(john, {item: 'apple', quantity: 1, unit_price_ 1}).to(be(null))

How to proceed now? keep the tests in the specs of Invoice, or move them to a brand new test file of InvoiceRowFactory ? Both cases feel wrong to be, because

  • if I keep the tests here, I'll be testing here something that belongs to another unit, that can and should be tested separately
  • if I move the tests, I'll be stubbing the dependency. At that point, in the future, I realise this code is horrible OOP, and want to replace InvoiceRowFactory with InvoiceRow. So a refactorer would love not to break anything while he's refactoring, keeping all the tests all green, but how? He can't, because my test of Invoice is too coupled with the InvoiceRowFactory stub

I just cannot reach the point where I can look at some code and make some changes keeping confident I'll not break anything, because I write such small units, that the only way to improve them is change the way they interact.

Thank you

Improve this question
0

3 Answers 3

Reset to default
6

If the tests feel wrong in either place, then you might be discovering an architectural problem with your code. Fixing the architectural problem will likely fix your test dilemma as well. Focus on this negative use case and write a test for it. Write the code you would like other code authors to use in this case. Should it throw an exception? Return null? In using TDD you get to discover these things as you write your tests. Since no code exists for this use case, you get to invent how the code looks.

So to answer your question, there is nothing wrong with your tests. By starting with TDD you discovered a flaw in your design. This is an opportunity to correct that design flaw by writing a test for this use case and defining how it should behave. This test will inform other code authors about what they should expect, and what error handling is necessary.

Improve this answer
5

The most direct implementation for a function like total does probably not require another component or class, that looks a little bit overdesigned to me at a first glance. But for the sake of this example, let us assume for a moment total would be something complex enough that is justifies the creation of such a second class.

I would recommend to go through the following steps:

  • first, implement function total(){return 0;} - should give you a failing test.

  • second, implement the summation in the constructor, store the result in some member variable, and return that member's content from total() - even if you think this is not the ideal design, and makes the constructor too big. Now, the test is "green".

  • third, and that is step which is pretty important in TDD: refactor. Now, you may decide to move parts of the logic in some separat class InvoiceRowFactory, and do the summation not in the constructor any more, but in total() (by utilizing InvoiceRowFactory).

It should be clear that for refactoring without breaking the existing code, one has to keep the tests inside the spec of Invoice. From this point of view, InvoiceRowFactory is not much more than an implementation detail of Invoice, so it does not necessarily require any tests on its own.

The former approach is a "top down" strategy of successive refinement. Alternatively, one can also decide to use a "bottom up" approach. In case one starts to foresee the need for something like an InvoiceRowFactory, maybe halfway in the middle of implementing total(), it is possible to postpone the creation of total(). For this, I would deactivate all tests for it for a moment, maybe comment out the half-baked code for total() (if that is necessary to get the code into a compilable state) and restart with InvoiceRowFactory first. In TDD, that means, of course, writing tests first, then implement InvoiceRowFactory.

When this class is complete, one can activate the tests for total() and the already written code for that function again, and finish it's implementation. This will end up with tests for both components, and InvoiceRowFactory more than just "an implementation detail", it has become now an independently reusable component.

Which of these route to prefer is often a judgement call, it depends on the specific situation, the complexity of the required components and also on your personal skills, experience and taste.

Improve this answer
4
  • Feels a bit like the bottom-up approach is not really TDD, since you thought about your implementation before even writing one single test. so a bottom-up approach would work, since the factory could be seen as a "private" helper. In this case, though, the test coverage would result lower than it actually is Commented May 7, 2020 at 6:48
  • 4
    @VecchiaSpugna: TDD is about writing tests before writing the code, not about writing tests before thinking about the design on a higher level of abstraction. In fact, there seems to be a popular misconception around that by TDD "the design emerges automatically"; that's pretty wrong. TDD only helps to design the API of a component, but not its inner structure. Commented May 7, 2020 at 7:07
  • 2
    And concerning the test coverage: that depends. When InvoiceRowFactory is really a component used for nothing else as as "private helper" of Invoice, with no extra functionality which could be of use somewhere else, changes are high the actual code and branch coverage, measured by a coverage tool, is the same in both cases. But if InvoiceRowFactory is designed to be a more general component, maybe called RowFactory, not specificially for invoices and with more functions than the ones needed for a summation, then extra tests with extra coverage make a lot of sense. Commented May 7, 2020 at 7:17
  • 1
    ... I meant "chances are high", sorry for the typo. Commented May 7, 2020 at 10:46
2

How to proceed now?

A common pattern is to create new tests for your new API, and then refactor the old implementation to use the new API.

So there is a period of time where there are "redundant" tests. For "programmer tests", meaning the tests that we re-run after each refactor to prove that we are still green, redundant tests aren't typically a big deal - tests without I/O are generally pretty fast to run anyway, and if the tests are isolated from each other you can run them in parallel.

As long as the required behavior hasn't changed, leaving the test in place is fine.

If you come to discover that you have made a test redundant, you can mark it as deprecated. That can act as a signal to your future self that you are confident that the verification of this requirement is handled elsewhere.

You retire the deprecated test when it gets in the way (because you want to retire the API it used, because the test is brittle, and so on).

In effect, tests have a draft->supported->deprecated->retired life-cycle similar to your implementations.

As long as your changes to the implementations are extensions (adding more stuff, as explained by Rich Hickey), the old tests can continue to run and pass.

What that normally looks like is making the api of the subject more flexible (optional arguments, a new named parameter, overloading) and having the original API delegate its work to the new one. This will often mean introducing a null object into your design to support the existing API

# Old version def do_the_thing(a,b,c): # generate value # New version def do_the_thing(a,b,c,d = None): if d is None: d = InertSubstitute() # generate value

Added bonus: if you learn how to extend your API in a backwards compatible way to support your tests, you'll also be learning how to extend your API in a way that is backwards compatible for the "real" clients of your API.

Improve this answer

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.