Chapter 11. Banking on Redesign

On the whole it’s worth evolving your design as your needs grow…1

Martin Fowler, Patterns of Enterprise Application Architecture

We introduced the Portfolio entity back in Chapter 3. It represents a key concept from our domain, so we’re justified in giving it some responsibility. Now, our Portfolio does too much work and it shows. Its primary job is to be a repository of Money s. However, it has taken on the added responsibility of converting between currencies. To do this, it has to hold on to an exchange rate table and the logic to do the conversion. This doesn’t look like the responsibility of a Portfolio. Monetary conversion has as much business being in a portfolio as peanut butter has being on top of a pizza.

Our software program has grown along with our needs. It is worth improving our design and looking for a better abstraction than the shoved-in way conversion between currencies is currently implemented.

A principle of Domain-Driven Design is continuous learning. When we learn something new about our domain, we let our design reflect our acquired knowledge. The resulting design and software should reflect our improved understanding of our domain.

Tip

“Domain-Driven Design" — or DDD — is a discipline that’s ably supported by TDD. Eric Evans’ book of the same title is the seminal work on the subject.

By implementing currency conversion over the last few chapters, we have gained fresh insight into our program. It’s missing a key entity. What is the name of the real world institution that helps us exchange money? A bank. Or a Currency Exchange. Often, a domain will have multiple similar entities that are indistinguishable from the perspective of our model. Learning which differences are salient and which are insignificant is vital to effective domain modeling.

We’ll select the name Bank to represent this missing entity. What should be the responsibilities of the Bank? It should hold exchange rates, for one thing. And it should be able to convert moneys between currencies based on the exchange rate from one currency to another. The Bank should allow asymmetric exchange rates, because that is true in the real world. Finally, the bank should clearly inform us when it cannot exchange money in one currency into a another currency because of a missing exchange rate. (Refer to the “Mixing moneys” section in Chapter 8 which lists ground rules for currency conversions.)

By being the entity that holds exchange rates, Bank will also deodorize our code. An odious smell is that the creation of keys for storing exchange rates — e.g. USD->EUR — is peppered throughout the Portfolio. This smell is a reliable indicator that we have a leaky abstraction. By keeping the exchange rate representation — keys and values — inside the Bank, we’ll simplify the way Portfolio performs evaluation.

Tip

When responsibilities spill over from one entity into another where they don’t belong, it’s called a leaky abstraction. In Joel Spolsky’s words: “All non-trivial abstractions, to some degree, are leaky.” However, gaping leaks should be plugged through refactoring.

Dependency Injection

Having identified the need for this new entity, the next question is: how should the dependencies between Bank and the other two existing entities — Money and Portfolio — look?

Clearly, Bank needs Money to operate. Portfolio would need both Money and Bank; the former association is one of aggregation and the latter is an interface dependency: Portfolio uses the convert method in Bank.

Figure 11-1 shows the three main entities in our program, their responsibilities, and their interdependencies.

The three main entities in our program: Portfolio, Bank, and Money
Figure 11-1. The three main entities in our program

The dependency of Portfolio to Bank is kept to a minimum: it is provided as a parameter to the Evaluate method. This type of dependency injection is called “Method Injection”, because we are “injecting” the dependency directly into the method that needs it.

Tip

Dependency Injection — the principle and practice of separating the initialization from the usage of a dependent entity — allows us to write loosely coupled code. There are several ways to inject a dependency, such as Constructor Injection, Property Injection, and Method Injection.

Putting it all together

We’re about to do some major surgery to our code — how will we ensure the health and well-being of our patient?

One key benefit of test-driven development is that long after the original code has been written, the tests provide anesthetic safety during later refactoring and redesign.

The approach we’ll take will be a combination of writing new unit tests — which is the heart of TDD and what we’ve done thus far — and refactoring existing unit tests. We know that the existing tests provide a valuable safeguard: they verify that the features we’ve built, all the crossed-out lines on our list, work as expected. We’ll continue to run these tests, modifying their implementation as needed while keeping their purpose intact. This two-pronged approach of writing new tests and refactoring existing ones will give us the assurance we need as we heal our code of its ills.

Important

Tests, especially unit tests, are a bulwark against regression failures during redesign.

With the theory and design out of the way, it’s time to write some code.

Go

Let’s write a test in money_test.go to convert one Money struct into another, using a yet-to-be-created Bank :

func TestConversion(t *testing.T) {
    bank := s.NewBank()
    bank.AddExchangeRate("EUR", "USD", 1.2)
    tenEuros := s.NewMoney(10, "EUR")
    actualConvertedMoney, err := bank.Convert(tenEuros, "USD")
    assertNil(t, err) 1
    assertEqual(t, s.NewMoney(12, "USD"), actualConvertedMoney)
}

func assertNil(t *testing.T, err error) { 2
    if err != nil {
        t.Errorf("Expected error to be nil, found: [%s]", err)
    }
}
1

Verifying that there is no error

2

New helper function that does the verification

Inspired by how NewMoney works, we create a Bank struct (DNEY: does not exist yet) by calling a NewBank function (DNEY). We call an AddExchangeRate function (DNEY) to add a particular exchange rate to the Bank. We then create a Money struct, and call Convert method (DNEY) to obtain another Money struct in a different currency. Finally, we assert that there is no error during the conversion and that the converted Money matches our expectation based on the exchange rate. We wrote the assertion as a new assertNil helper function after the pattern established by the existing assertEqual function.

If there are too many concepts (structs and methods) that do not exist yet, it’s because we choose to go faster. We could always slow down and write smaller tests as we did early on in our journey, if we wanted.

Tip

Using test-driven development, we can write tests that introduce several new concepts — and thereby go faster.

We wrote our test with the assumption that the Convert method (DNEY) in Bank will have two return types: a Money and an error. Why did we change the signature of this Convert method from the convert method that already exists in Portfolio and returns a float64 and a bool? Because conceptually, the Bank converts money in one currency to money in another currency. The first return value, therefore, is Money and not just a float64 expressing an amount. The second return value is an error so that we can use it to indicate the exchange rate for the failed conversion, which cannot be done with a mere bool.

We are doing just-enough design based on what we know now. We’re neither speculating (over-designing) nor dumbing things down (under-designing).

To make this test green, we need to craft all the things that DNEY — do not exist yet. Let’s create a new source file in the stocks package named bank.go:

package stocks

import "errors"

type Bank struct {
    exchangeRates map[string]float64
}

func (b Bank) AddExchangeRate(currencyFrom string, currencyTo string,
        rate float64) {
    key := currencyFrom + "->" + currencyTo
    b.exchangeRates[key] = rate
}

func (b Bank) Convert(money Money, currencyTo string) (convertedMoney Money,
        err error) {
    if money.currency == currencyTo {
        return NewMoney(money.amount, money.currency), nil 1
    }
    key := money.currency + "->" + currencyTo
    rate, ok := b.exchangeRates[key]
    if ok {
        return NewMoney(money.amount*rate, currencyTo), nil 1
    }
    return NewMoney(0, ""), errors.New("Failed") 2
}

func NewBank() Bank {
    return Bank{exchangeRates: make(map[string]float64)}
}
1

When conversion is successful, a Money and a nil (no error) are returned

2

When conversion fails, a placeholder Money and an error are returned

We introduce the missing concepts:

  1. A type named Bank.

  2. The Bank struct containing a map to store exchangeRates.

  3. A function NewBank to create structs of type Bank.

  4. A method named AddExchangeRate that stores exchange rates needed to convert Money s.

  5. A method named Convert that is largely similar to the existing convert method in Portfolio. The return values are a Money and an error. If the conversion is successful, a Money is returned and the error is nil. If the conversion fails due to a missing exchange rate, a placeholder Money object and an error are returned.

With these changes carefully done, our new test passes.

We know that the existing behavior of Evaluate — which we need to retain — returns an error with all the missing exchange rates. Where will these missing exchange rates emanate from? The Convert method, when we use it in Evaluate shortly. This means that the error returned from Convert must include the missing exchange rate. We have it readily available in the Convert method: it’s in the variable named key. Even though replacing the hard-coded error message "Failed" with the variable key is a small change, let’s test-drive it. Why? It would also allow us to address the snotty little smell of returning the placeholder Money struct when there is an error. It’s better to have the two return values be symmetrical: convertedMoney should hold the result of conversion when err is nil and when the latter describes a conversion error, the former is nil.

Tip

Go’s standard libraries have functions and methods (e.g. os.Open(), http.PostForm(), and parse.Parse()) that return a pointer for the first return value and an error for the second. While this is not a strictly enforced language rule, it is the link:https://blog.golang.org/go1.13-errors#TOC_5.[style described in the Go blog].

Let’s write a second test to drill the proper error message and this symmetry into our Convert method.

func TestConversionWithMissingExchangeRate(t *testing.T) {
    bank := s.NewBank()
    tenEuros := s.NewMoney(10, "EUR")
    actualConvertedMoney, err := bank.Convert(tenEuros, "Kalganid") 1
    if actualConvertedMoney != nil { 2
        t.Errorf("Expected money to be nil, found: [%+v]", actualConvertedMoney)
    }
    assertEqual(t, "EUR->Kalganid", err.Error()) 3
}
1

Converting Money in EUR to Kalganid

2

Asserting that a nil Money pointer is returned

3

Asserting that returned error contains the missing exchange rate

This test, TestConversionWithMissingExchangeRate, attempts to convert Euros to Kalganid — a currency for which no exchange rate has been defined in the Bank. We expect the two return values from Convert to be a Money pointer that’s nil and an error that contains the missing exchange rate.

This test fails to compile because of mismatched types:

... invalid operation: actualConvertedMoney != nil
        (mismatched types stocks.Money and nil)

This counts as a failing test. To allow nil values to be returned from Convert, we change the type of the first return value to be a pointer.

func (b Bank) Convert(money Money, currencyTo string) (convertedMoney *Money, 1
        err error) {
    var result Money
    if money.currency == currencyTo {
        result = NewMoney(money.amount, money.currency)
        return &result, nil 2
    }
    key := money.currency + "->" + currencyTo
    rate, ok := b.exchangeRates[key]
    if ok {
        result = NewMoney(money.amount*rate, currencyTo)
        return &result, nil 2
    }
    return nil, errors.New("Failed") 3
}
1

First return type is now a Money pointer

2

When conversion is successful, a valid pointer to Money and a nil error are returned

3

When conversion fails, a nil Money pointer and an error with missing exchange rate are returned

Running this test now gives us a failure message in an older test, TestConversion, reminding us about dereferencing pointers:

=== RUN   TestConversion
    ...   Expected  [{amount:12 currency:USD}]
               Got: [&{amount:12 currency:USD}] 1
1

Expected a Money, got a Money pointer

That little ampersand & makes a world of difference! The actualConvertedMoney variable in TestConversion is now a pointer to a Money and needs to be dereferenced:

    assertEqual(t, stocks.NewMoney(12, "USD"), *actualConvertedMoney) 1
1

The * dereferences the actualConvertedMoney pointer back into the struct it points to

With these changes, we get the assertion failure we are chasing:

=== RUN   TestConversionWithMissingExchangeRate
    ...   Expected  [EUR->Kalganid] Got: [Failed]

Replacing the string "Failed" with key in the Convert method gets the test to pass.

    return nil, errors.New(key) 1
1

Last line of Convert method

We’re in the REFACTOR phase. One thing we can improve is our assertNil method. It’s only good for verifying if an error is nil; if it could take any type (like assertEqual already does) we could use it to assert that the Money pointer is nil, too.

Let’s try an implementation for assertNil following the lead of assertEqual:

func TestConversionWithMissingExchangeRate(t *testing.T) {
...
    assertNil(t, actualConvertedMoney) 1
...
}

func assertNil(t *testing.T, actual interface{}) { 2
    if actual != nil {
        t.Errorf("Expected to be nil, found: [%+v]", actual) 3
    }
}
1

Using the modified assertNil to verify a nil pointer

2

Using empty interface to represent (almost) anything

3

Using the %+v verb to print non-nil values

With this implementation in use, we get an rather strange failure when we run our tests.

=== RUN   TestConversionWithMissingExchangeRate
    money_test.go:108: Expected to be nil, found: [<nil>]

That’s puzzling! If nil was expected and <nil> was found, what’s the problem?

Those little angle-brackets give us a clue. In Go, interfaces are implemented as two elements: a type T and a value V. The way Go stores a nil inside an interface means that only V is nil whereas T is a pointer to whatever type the interface represents (a *Money in our case). Since the type T is not nil, the interface itself is also not nil.

To use an analogy: an interface is like a wrapping paper and the box surrounding a gift. You have to rip it open to see what the gift is. It is possible that there is nothing inside the box — the ultimate gag gift — but you can’t find that out until you unwrap and unbox the gift first.

Important

A Go interface will be non-nil even when the pointer value inside it is nil.

The way to unwrap an interface and examine the value inside is to to use the reflect package. The ValueOf function in that package returns the value V, which can then be checked by calling the IsNil function, also defined in the the reflect package. To avoid panic errors from inspecting nil interfaces, we must first check if the given interface{} is nil, too.

Here’s what the corrected assertNil function looks like.

import (
    s "tdd/stocks"
    "testing"
    "reflect" 1
)
...
func assertNil(t *testing.T,actual interface{}) {
    if actual != nil && !reflect.ValueOf(actual).IsNil() { 2
        t.Errorf("Expected to be nil, found: [%+v]", actual)
    }
}
1

We need the reflect package to examine the interface

2

The assertion error is raised if neither the interface{} itself is nil nor is the wrapped value

Excellent! Our tests all pass and we have the symmetric Convert method we set out to write. We’re ready to introduce Bank as a dependency of the Evaluate method in Portfolio.

Since we have a battery of tests for the Evaluate method, we’ll sally forth and redesign that method now. Any test failures we get — and we do expect to get many — will keep us on the RGR track.

We change the signature of the Evaluate method in Portfolio to have a Bank as the first parameter. We also change the type of its first return value to be a Money pointer. As for the body of the method, the changes are few and specific. We call Bank’s Convert method instead of the soon-to-be-retired local function convert. Whenever a call to Convert returns an error, we save the error message. Instead of returning a Money struct, we return a pointer to it. And when there are errors, we return a nil as the first value and the error as the second.

func (p Portfolio) Evaluate(bank Bank, currency string) (*Money, error) { 1
    total := 0.0
    failedConversions := make([]string, 0)
    for _, m := range p {
        if convertedCurrency, err := bank.Convert(m, currency); err == nil {
            total = total + convertedCurrency.amount
        } else {
            failedConversions = append(failedConversions, err.Error())
        }
    }
    if len(failedConversions) == 0 {
        totalMoney := NewMoney(total, currency)
        return &totalMoney, nil 2
    }
    failures := "["
    for _, f := range failedConversions {
        failures = failures + f + ","
    }
    failures = failures + "]"
    return nil, errors.New("Missing exchange rate(s):" + failures) 3
}
1

A Money pointer and an error are returned

2

When the Money pointer is not nil, a nil error is returned

3

When there is an error, the a nil Money pointer is returned

With its signature changed, every call to Evaluate in our test fails to compile. We need to create a Bank and pass it to Evaluate. Could we do it once in money_test.go, and not within each individual test method?

Absolutely! Let’s declare a Bank variable outside all tests in money_test.go and use an init function to initialize this Bank with all necessary exchange rates:

var bank s.Bank 1

func init() { 2
    bank = s.NewBank()
    bank.AddExchangeRate("EUR", "USD", 1.2)
    bank.AddExchangeRate("USD", "KRW", 1100)
}
1

In money_test.go, outside any test method

2

New init function

Tip

There are multiple ways to set up shared state in Go. Each test file can have one or more init() functions, which are executed in order. All init functions must have identical signature. Alternatively, we can override the MainStart function in a test file and call one (or more) setup/teardown methods, which may have arbitrary signature.

Now we can use this bank in each call to Evaluate in our tests, e.g.: portfolio.Evaluate(bank, "Kalganid").

Since Evaluate now returns a Money pointer and an error, we have to change how we assign these return values to variables and how we assert for them.

Here’s how our TestAddition looks after making the necessary changes:

func TestAddition(t *testing.T) {
    var portfolio s.Portfolio

    fiveDollars := s.NewMoney(5, "USD")
    tenDollars := s.NewMoney(10, "USD")
    fifteenDollars := s.NewMoney(15, "USD")

    portfolio = portfolio.Add(fiveDollars)
    portfolio = portfolio.Add(tenDollars)
    portfolioInDollars, err := portfolio.Evaluate(bank, "USD") 1

    assertNil(t, err) 2
    assertEqual(t, fifteenDollars, *portfolioInDollars) 3
}
1

Injecting the bank dependency into the Evaluate method

2

Asserting that there is no error

3

Dereferencing pointer to Money before using it as the last parameter in assertEqual function

After fixing the other tests similarly — using bank as the first parameter to Evaluate and dereferencing the pointer to get a reference to Money — we get all tests to pass. We’re now ready to remove the unused convert function in Portfolio. Deleting unused code is such a gratifying feeling!

Since we have a general-purpose and robust assertNil function available to our tests, we replace all blank identifiers _ with actual variables and verify that they’re nil. For example, in TestAdditionWithMultipleMissingExchangeRates, we can verify that the Money pointer is nil:

func TestAdditionWithMultipleMissingExchangeRates(t *testing.T) {
...
    expectedErrorMessage :=
        "Missing exchange rate(s):[USD->Kalganid,EUR->Kalganid,KRW->Kalganid,]"
    value, actualError := portfolio.Evaluate(bank, "Kalganid") 1

    assertNil(t, value) 2
    assertEqual(t, expectedErrorMessage, actualError.Error())
}
1

Receive the first return value, a Money pointer, in a named parameter instead of a blank identifier

2

Verify that a nil Money pointer is returned

We now have a much better code organization: Bank, Portfolio, and Money have specific responsibilities. We have robust tests that validate the behavior of all these types. A good indicator of our improved code is that each file in the stocks package is of comparable size: a few dozen lines of code. (If we write Godoc comments — which is a great thing — the files would be longer.)

JavaScript

Let’s write a new test in test_money.js to convert one Money object into another, using the Bank class that we intend to create:

const Bank = require('./bank'); 1
...
    testConversion() {
        let bank = new Bank(); 2
        bank.addExchangeRate("EUR", "USD", 1.2); 3
        let tenEuros = new Money(10, "EUR");
        assert.deepStrictEqual(
            bank.convert(tenEuros, "USD"), new Money(12, "USD")); 4
    }
1

Import statement for bank module (Does Not Exist Yet)

2

Call to Bank constructor (DNEY)

3

Call to addExchangeRate (DNEY)

4

Call to convert (DNEY)

Notice that the module being imported, the class Bank, and its methods addExchangeRate and convert do not exist yet (DNEY).

Anticipating (or observing) the test failures, let’s create a new file named bank.js containing the requisite behavior of the Bank class.

const Money = require("./money"); 1

class Bank {
    constructor() {
        this.exchangeRates = new Map(); 2
    }

    addExchangeRate(currencyFrom, currencyTo, rate) {
        let key = currencyFrom + "->" + currencyTo; 3
        this.exchangeRates.set(key, rate);
    }

    convert(money, currency) { 4
        if (money.currency === currency) {
            return new Money(money.amount, money.currency); 5
        }
        let key = money.currency + "->" + currency;
        let rate = this.exchangeRates.get(key);
        if (rate == undefined) {
            throw new Error("Failed"); 6
        }
        return new Money(money.amount * rate, currency);
    }
}

module.exports = Bank; 7
1

The Bank class requires the Money class

2

Create an empty Map in constructor for use later

3

Forming a key to store the exchange rate

4

This convert method resembles the convert method in Portfolio class

5

Creating a new Money object when currencies are the same

6

When exchange rate is undefined, throw an Error with the string “Failed”

7

Exporting Bank class for use outside this module

The addExchangeRate creates a key using the “from” and “to” currencies and stores the rate using this key.

Most of the behavior of convert method is heavily influenced by the existing code in Portfolio class. The one difference is that Bank.convert returns a Money object when successful (instead of only the amount) and throws an Error upon failure (instead of returning undefined). Additionally, the Bank.convert method always creates a new Money object, even when the “from” and “to” currencies are identical. This prevents accidental side-effects by always returning a new Money object, never the one that’s the first parameter to the method.

Important

Javascript objects (and arrays) are passed by reference. If we need to simulate pass-by-value semantics — important to reduce side-effects — we must create new objects explicitly.

The tests are all green.

We need to retain the existing behavior of evaluate, which returns an Error with all the missing exchange rates. The evaluate method will need this new convert method to provide all those missing exchange rates. Therefore, the Error thrown from convert must include the missing exchange rate. We have this value already available in the key variable of the Bank.convert method. Even though this is a small change, let’s test-drive it.

We add a new test in test_money.js to verify the behavior we need.

  testConversionWithMissingExchangeRate() {
    let bank = new Bank(); 1
    let tenEuros = new Money(10, "EUR");
    let expectedError = new Error("EUR->Kalganid"); 2
    assert.throws(function () { bank.convert(tenEuros, "Kalganid") },
      expectedError); 3
  }
1

New Bank with no exchange rates defined

2

Expected error include the missing exchange rate

3

Using anonymous function with assert.throws to verify the error message

We use the same assert.throws idiom, using an anonymous function, that we did in Chapter 10 while writing testAdditionWithMultipleMissingExchangeRates.

This test fails with the expected assertion failure:

Running: testConversionWithMissingExchangeRate()
AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected

  Comparison {
+   message: 'Failed',
-   message: 'EUR->Kalganid',
    name: 'Error'
  }

Getting this test to pass is simple: we use key when throwing Error from convert :

    convert(money, currency) {
...
        if (rate == undefined) {
            throw new Error(key); 1
        }
...
    }
1

Using key ensures the Error includes the missing exchange rate

With these changes to the Bank class, all our tests passes. We’re ready to change the Portfolio class. Because we have a suite of tests for Portfolio, we’ll jump into the redesign. We trust our tests — and their anticipated failures — to ensure that we do the redesign correctly.

The evaluate function should accept a Bank object as a dependency. We’ll put this as the first parameter to the evaluate method. The rest of the method is modified to work with the Error that Bank.convert throws. The error message wraps the missing exchange rate, so we can keep a record of any missing exchange rates and throw one Error with the aggregated error message from evaluate, when necessary.

    evaluate(bank, currency) {
        let failures = [];
        let total = this.moneys.reduce( (sum, money) => {
            try {
                let convertedMoney = bank.convert(money, currency); 1
            }
            catch (error) {
                failures.push(error.message);
                return sum;
            }
            return sum + convertedMoney.amount;
          }, 0);

        if (failures.length == 0) {
            return new Money(total, currency);
        }
        throw new Error("Missing exchange rate(s):[" + failures.join() + "]");
    }
1

Calling Bank’s convert method

Tip

JavaScript has the throw keyword to signal an exception; and try ... catch and try ... catch ... finally constructs to respond to exceptions.

Since we have changed the signature of evaluate, we have no justifiable hope that any of our addition tests will pass. Just for giggles, we run the test suite as it is. When we run it, we get a strange-looking error:

Running: testAddition()
...
Error: Missing exchange rate(s):[bank.convert is not a function,
  bank.convert is not a function]

That looks odd: whatever shortcomings it may have, bank.convert is surely a function since we just wrote it! The reason for this error message is that our test calls evaluate with only one parameter, and JavaScript’s rules allow this. The currency string is assigned to the first parameter, and the second parameter is set to undefined, as shown in Figure 11-2:

In JavaScript any missing parameters in a method call are left as undefined
Figure 11-2. In JavaScript, any missing parameters in a method call are left as undefined
Important

JavaScript does not enforce any rules on the number or types of parameters that are passed to a function, regardless of what the function definition states.

Even though it’s assigned to the parameter named bank, the first parameter’s value is a mere string. The Node.js runtime is justified in saying that it doesn’t have a convert method.

Let’s create a Bank object and pass it as the first parameter to the evaluate method. This is sufficient for testAddition, which uses the same currency throughout (i.e. no exchange rates required).

  testAddition() {
    let fiveDollars = new Money(5, "USD");
    let tenDollars = new Money(10, "USD");
    let fifteenDollars = new Money(15, "USD");
    let portfolio = new Portfolio();
    portfolio.add(fiveDollars, tenDollars);
    assert.deepStrictEqual(portfolio.evaluate(new Bank(), "USD"),
      fifteenDollars); 1
  }
1

No exchange rates needed in this test, just a Bank object

The testAddition passes. The failure message progresses to the next test in our test suite.

We need to fix the other tests similarly, albeit with exchange rates defined. Since we’re going to need a Bank object and exchange rates in multiple tests, it’d be nice to only define the Bank once with all the exchange rates needed for our tests. We can define bank as a member variable and initialize it in a MoneyTest constructor:

    constructor() {
        this.bank = new Bank();
        this.bank.addExchangeRate("EUR", "USD", 1.2);
        this.bank.addExchangeRate("USD", "KRW", 1100);
    }

In most of the addition tests, we can use this.bank directly, without having to create a method-local bank. For example:

  testAdditionOfDollarsAndEuros() {
...
    assert.deepStrictEqual(portfolio.evaluate(this.bank, "USD"),
      expectedValue); 1
  }
1

[.small]#The bank object created in the constructor is accessible as this.bank #

The one exception to using this.bank is testAdditionWithMultipleMissingExchangeRates, where we deliberately seek to cause an error. Because the parameter to the assert statement in this test is an anonymous function object, the reference to this.bank will fail … because this has changed!

Let’s untangle the previous paragraph. When we create an object, say: ABC in JavaScript, any code within ABC can use this to refer to ABC. Any objects outside ABC cannot be accessed using this.

Important

In JavaScript, this refers to the nearest enclosing object, including anonymous objects, and not any other objects surrounding it.

We need to get a local reference to the bank and use it in the assertion inside testAdditionWithMultipleMissingExchangeRates:

        let bank = this.bank; 1
        assert.throws(function() {portfolio.evaluate(bank, "Kalganid")},
          expectedError); 2
1

Saving the reference to this.bank in a local variable

2

[.small]#Using local variable bank because this means something different inside the anonymous function #

Yay: green tests! We’re now ready to remove the unused convert function in Portfolio. Deleting unused code is pure exhilaration!

We now have well organized code: Bank, Portfolio, and Money have specific responsibilities. A good indicator is that each file is now of comparable size.

Python

Our first goal is to write a test to convert one Money object into another, using the as-yet-undefined Bank abstraction:

from bank import Bank 1
...
    def testConversion(self):
        bank = Bank() 2
        bank.addExchangeRate("EUR", "USD", 1.2) 3
        tenEuros = Money(10, "EUR")
        self.assertEqual(bank.convert(tenEuros, "USD"), Money(12, "USD")) 4
1

Import statement for bank module (Does Not Exist Yet)

2

Create a new Bank` (DNEY)

3

Call to addExchangeRate (DNEY)

4

Call to convert (DNEY)

We are using several things that do not exist yet (DNEY): the bank module, the Bank class, and the addExchangeRate and convert methods in the Bank class.

Anticipating (or observing) the test errors we get — such as ModuleNotFoundError: No module named 'bank' — let’s create a new file named bank.py which defines the Bank class with the minimal necessary behavior.

from money import Money 1

class Bank:
    def __init__(self):
        self.exchangeRates = {} 2

    def addExchangeRate(self, currencyFrom, currencyTo, rate):
        key = currencyFrom + "->" + currencyTo 3
        self.exchangeRates[key] = rate

    def convert(self, aMoney, aCurrency): 4
        if aMoney.currency == aCurrency:
            return Money(aMoney.amount, aCurrency) 5

        key = aMoney.currency + "->" + aCurrency
        if key in self.exchangeRates:
            return Money(aMoney.amount * self.exchangeRates[key], aCurrency)

        raise Exception("Failed") 6
1

The Bank requires Money as a dependency

2

Initializing empty dictionary in __init__ method

3

Forming a key to store the exchange rate

4

This convert method resembles the __convert method in the Portfolio class

5

Creating a new Money object when currencies are the same

6

raising an Exception when conversion fails

The Bank class — especially its convert method — borrows generously from the existing code in Portfolio. The two key differences are in the signature of Bank.convert. It returns a Money object when successful (instead of merely an amount) and raises a general Exception when the conversion fails (instead of a KeyError).

With this new Bank class, our test passes.

We need to keep the existing behavior of evaluate, which returns an Exception with all the missing exchange rates. The evaluate method will need the convert method to provide the missing exchange rates. The Exception raised from convert must include the missing exchange rate — the value is in the key variable in convert. Let’s test-drive this change, small though it is.

We write a new test that expects an Exception with a specific message from the convert method:

    def testConversionWithMissingExchangeRate(self):
        bank = Bank() 1
        tenEuros = Money(10, "EUR")
        with self.assertRaisesRegex(Exception, "EUR->Kalganid"): 2
            bank.convert(tenEuros, "Kalganid") 3
1

New Bank with no exchange rates defined

2

Expected Exception with specific message

3

Call to convert with "Kalganid" as the currency

This test fails as expected:

FAIL: testConversionWithMissingExchangeRate (__main__.TestMoney)
...
Exception: Failed
...
AssertionError: "EUR->Kalganid" does not match "Failed"

To fix this, we use key to create the Exception that’s raised from convert:

    def convert(self, aMoney, aCurrency):
...
        raise Exception(key) 1
1

Using key ensures the Exception includes the missing exchange rate

All tests are green. With the new Bank class in place, we’re ready to change the evaluate method in Portfolio to accept a Bank object as a dependency. We have no fewer than four tests for addition of Money s which exercise the evaluate method. We fully expect these tests to fail, thereby keeping us firmly on the RGR track.

We’ll place bank as the second method parameter to evaluate, after the obligatory self. The rest of the method is modified to work with the Exception that Bank.convert throws when there’s a missing exchange rate.

    def evaluate(self, bank, currency):
        total = 0.0
        failures = []
        for m in self.moneys:
            try:
                total += bank.convert(m, currency).amount 1
            except Exception as ex:
                failures.append(ex)

        if len(failures) == 0:
            return Money(total, currency)

        failureMessage = ",".join(f.args[0] for f in failures)
        raise Exception("Missing exchange rate(s):[" + failureMessage + "]")
1

Delegate to bank.convert method

As soon as we we make these changes to evaluate, several of our tests fail with a rather strange error:

TypeError: evaluate() missing 1 required positional argument: 'currency'

That’s strange: the only “positional argument” we’re passing in is the currency; what’s missing is the bank! The reason for this rather phantasmagoric error message is that Python is a dynamically typed language. It assumes that the first (and only) argument we’re passing corresponds to the first positional argument in the evaluate method declaration. Since it doesn’t find a second argument matching currency — it complains about it.

Figure 11-3 shows how actual parameters are associated with the formal parameters in a Python method call.

In Python, parameters are assigned left-to-right based on position, regardless of their type
Figure 11-3. In Python, parameters are assigned left-to-right based on position, regardless of their type

We need a Bank with a couple of exchange rates to satisfy the needs of all our addition-related tests.

It’d be nice if we could declare this initialization code once, rather than in each test. There is a way to do this. Our test class, by virtue of subclassing from unittest.TestCase, inherits its behavior. One aspect of this inherited behavior is that if there is a setUp method in the class, it’ll be called before each test. We can define our Bank object in this setUp method:

    def setUp(self): 1
        self.bank = Bank() 2
        self.bank.addExchangeRate("EUR", "USD", 1.2)  3
        self.bank.addExchangeRate("USD", "KRW", 1100) 3
1

Overridden setUp method from TestCase superclass

2

New Bank object needed by tests

3

Exchange rates needed by tests

In the test methods, we can simply use self.bank as the first argument to each call to evaluate, for example:

    def testAddition(self):
...
        self.assertEqual(fifteenDollars, portfolio.evaluate(self.bank, "USD")) 1
1

Using self.bank declared in the setUp method

After fixing all calls to evaluate in this way, the tests are pristine green. The scene is set for us to ceremoniously delete the old __convert method in Portfolio. Deleting code is a sweet feeling: savor it!

Committing our changes

This has been a sizeable modification to our code: the introduction of Bank and the resultant refactoring. Let’s commit our code to our local Git repository, with a message that reflects what we did:

git add .
git commit -m "refactor: added Bank; refactored Portfolio to use Bank"

Where We Are

We changed the internal organization of our code in a significant way, extracting the Bank entity from the obscurity of being embedded in Portfolio into a first-class citizen of our domain. And we used a combination of new tests and our existing suite of tests to ensure that no features were harmed during this writing of the new and improved code. We also cleaned up our tests by declaring a Bank variable once before the tests run and then using this instance in the relevant tests.

We have one more item on our list: the ability to modify existing exchange rates. Before we get to that, let’s add the satisfaction of crossing one more item off our list to the sumptuous pleasure that redesigning and deleting code has provided for us.

5 USD x 2 = 10 USD

10 EUR x 2 = 20 EUR

4002 KRW / 4 = 1000.5 KRW

5 USD + 10 USD = 15 USD

Separate test code from production code

Remove redundant tests

5 USD + 10 EUR = 17 USD

1 USD + 1100 KRW = 2200 KRW#

Determine exchange rate based on the currencies involved (from → to)

Improve error handling when exchange rates are unspecified

Improve the implementation of exchange rates

Allow exchange rates to be modified

1 Stated in the context of Object-Relational Metadata Mapping. However, it’s good advice in general.

..................Content has been hidden....................

You can't read the all page of ebook, please click here login for view all page.
Reset