Profile

To gather complexity metrics for our JavaScript code, we can use a tool like JSHint. JSHint comes in many guises, the home page provides an online editor in which you can paste JavaScript code and measure its complexity. For our purposes, the NodeJS package is more appropriate. JSHint can be installed globally by running npm install -g jshint at a command line.

To use jshint, we need to specify a couple of configuration parameters. The simplest way to do this is to create a file named .jshintrc in the js folder.

{
    "esversion"     : 6, 
    "maxcomplexity" : 1  
}

Specifying the version of EcmaScript to use

Setting the maximum cyclomatic complexity to the lowest possible value

Note that we have set the maxcomplexity to 1 — the lowest possible cyclomatic complexity for any method. This goal isn’t to meet this threshold: as mentioned earlier, 10 is a more typical value. The reason we set it to 1 here is to force jshint to ptint every method with a higher cyclomatic complexity as an error.

With this short .jshintrc file in place, we can simply run jshint from the command line in the js folder to examine which methods have a cyclomatic complexity that exceeds 1.

bank.js: line 13, col 12, This function's cyclomatic complexity is too high. (3)

portfolio.js: line 13, col 42, This function's cyclomatic complexity is too high. (2)
portfolio.js: line 11, col 13, This function's cyclomatic complexity is too high. (2)

test_money.js: line 81, col 44, This function's cyclomatic complexity is too high. (2)
test_money.js: line 89, col 30, This function's cyclomatic complexity is too high. (3)

We see that there a couple of methods with a cyclomatic complexity of three, and a few more with a complexity metric of two.

This validates a key claim of Test-Driven Development. The incremental and evolutionary style of coding that TDD encourages results in code with a more uniform complexity profile. Instead of one or two “superman” methods or classes, we get a better distribution of responsibility across our modules.

The coupling between our code is low. Both Portfolio and Bank depend on Money, which is a natural consequence of our domain. There is also a more subtle dependency from Bank to Portfolio. It’s subtle, because unlike Money, the Portfolio does not require an object of type Bank. The evaluate method in Portfolio requires a “bank like object" — that is, an object that implements a convert method. This is a dependency on an interface and not a specific implementation. This is different from how Portfolio depends on Money: there is an explicit call to new Money() in the evaluate method.

We encountered Dependency Injection in Chapter 4 and Chapter 11. We can inject any object that implements a convert method to test Portfolio.evaluate — it doesn’t have to be an actual Bank object. Consider this strangely written but valid — and passing — test.

testAdditionWithTestDouble() {
  const moneyCount = 10; 
  let moneys = []
  for (let i = 0; i < moneyCount; i++) {
    moneys.push(new Money(Math.random(Number.MAX_SAFE_INTEGER), "Does Not Matter")); 
  }
  let bank = { 
    convert: function() { 
      return new Money(Math.PI, "Kalganid"); 
    }
  };
  let arbitraryResult = new Money(moneyCount * Math.PI, "Kalganid"); 

  let portfolio = new Portfolio();
  portfolio.add(...moneys);
  assert.deepStrictEqual(portfolio.evaluate(bank, "Kalganid"), arbitraryResult); 
}

Number of Money objects in our test

Each Money object has a random amount and its currency is also made up

A Bank test double

Overriden convert method

Always return “π Kalganid”

The result is expected to be “π times moneyCount" Kalganid

Assertion, which passes

We have created a silly implementation of a bank in our test. Silly from a business standpoint, yet completely valid from an interface perspective. This bank always returns “π Kalganid” from its convert method, regardless of any arguments it’s given. ² This means that for each call to this convert method from Portfolio.evaluate, the Portfolio accumulates “π Kalganid”. Thus the final result is π times the number of Money objects, in Kalganid.

Even though the test is peculiar, it illustrates the key concepts of “Test Doubles” and Interface Dependency.

It’s obvious that we could follow the pattern shown in the preceding test and rewrite all our tests for Portfolio.convert to use test doubles instead of the “real” Bank. The real question is: should we?

The answer is not obvious. As a general rule: use the path of least resistance. If the effort to introduce a test double is greater than using the real code, then use the real code. Otherwise, use a test double.

There is also a risk of using a test double: if there are non-obvious side effects between the system under test and the the dependency, a test double may inadvertently mask these side effects. Or the test double may introduce new side effects that are not present in the real code. Either way, there is a risk that testing with the test double may not be a faithful replica of testing with the “real” dependency in place.

Is there a way out of this? Using stateless code with a well-defined interface is a start. A method that is stateless — that is, one whose behavior depends completely on its parameters — is much easier to replace with a test double than a method which relies heavily on mutable state of surrounding objects that are not passed as parameters. ³

Purpose

The three classes in our JavaScript display singularity of purpose: a class for each key concept.

Is there something worth improving? There is a bit of a leaky abstraction in the try block in the Portfolio.evaluate method.

 try {
     let convertedMoney = bank.convert(money, currency);
     return sum + convertedMoney.amount;
 }

Did you spot it? The conversion of money into a new currency yields a compact, self-contained object: convertedMoney. We then pry open this object to look at its amount and add it to a total … only for us to later put Humpty Dumpty back again when we return new Money(total, currency) at the end of the method!

How would our code look if the Money class had an add method? Specifically, how might we drive it through tests, and what would the refactored Portfolio.evaluate look like?

We can think of a few tests we may use to drive out the behavior of Money.add. Adding two Money objects in the same currency should work in a straightforward way, obeying the commutative law of addition of numbers. Adding two Money objects with different currencies should fail with an appropriate exception. We can justify this exceptional behavior on the grounds that adding multiple currencies requires us to maintain a Portfolio — which already carries the responsibility of conversion.

testAddTwoMoneysInSameCurrency() { 
  let fiveKalganid = new Money(5, "Kalganid");
  let tenKalganid = new Money(10, "Kalganid");
  let fifteenKalganid = new Money(15, "Kalganid");
  assert.deepStrictEqual(fiveKalganid.add(tenKalganid), fifteenKalganid;
  assert.deepStrictEqual(tenKalganid.add(fiveKalganid), fifteenKalganid;
}

testAddTwoMoneysInDifferentCurrencies() { 
  let euro = new Money(1, "EUR");
  let dollar = new Money(1, "USD");
  assert.throws(function() {euro.add(dollar);},
    new Error("Cannot add USD to EUR"));
  assert.throws(function() {dollar.add(euro);},
    new Error("Cannot add EUR to USD"));
}

Test to verify directly adding two Money objects in same currency

Test to verify exception when attempting to add two Money objects with different currencies

After we write a Money.add method that fulfils the above tests, ⁴ we can use it to reduce the leaky abstraction in Portfolio.evaluate.

evaluate(bank, currency) {
    let failures = [];
    let total = this.moneys.reduce( (sum, money) => {
        try {
            let convertedMoney = bank.convert(money, currency);
            return sum.add(convertedMoney); 
        }
        catch (error) {
            failures.push(error.message);
            return sum;
        }
      }, new Money(0, currency)); 
    if (failures.length == 0) {
        return total; 
    }
    throw new Error("Missing exchange rate(s):[" + failures.join() + "]");
}

Using the Money.add method

The initial value is a Money object, not a number

The total can be returned directly: it’s a Money object

Is the cost of removing the leaky abstraction worth the extra method in Money class and the tests for it?

There isn’t a cut-and-dried answer for it. We could reason that the add method is a worthy companion to the times and divide methods already in Money and that preventing the Portfolio.evaluate method from prying into Money is a good thing. On the other hand, we could reason that Bank.convert already pries into both the currency and amount of the Money object it’s given; and that there’s no obvious way to remove that leaky abstraction without adding substantially more behavior to Money — at the expense of Bank, probably.

The contrasting answers are reflective of the element of subjectivity inherent in the notion of “fit for purpose”. It’s reasonable to have different opinions on the question of “what is the purpose of this class?” The code that results from the different opinions will also be different — invariably and unavoidably.

Process

In the beginning, all our source code was in one file. We introduced separation of concerns in Chapter 4 and used it to partition our code into modules in Chapter 6. The Bank class was introduced in Chapter 11. It’s likely that our code would have turned out differently had we followed another path. Could it have been better?

If we were to solve the problem again, we may separate the tests from the production code earlier — perhaps as soon as we have one green test. Early separation of concerns can have benefits: it forces us to make our dependencies explicit and think critically about what each module exports. This can lead to better encapsulation (i.e. information hiding).

Can you think of shortcomings in our JavaScript code? Which steps during the incremental growth of our code caused them? And were there any stages where we could have corrected them?