Find currency exchange rates

Question

Description:

Design a service to fetch exchange rate from a remote resource and then calculate the exchange rate for each currency pair.

The remote resource contains the exchange rates of each currency in Euro.

This is an interview assignment and I came up with a simple solution.

index.js

'use strict';

const joi = require('joi');

const api = require('./api');
const Exchange = require('./exchange');
const xmlParser = require('./parse-xml');

const schema = joi
  .object({
    source: joi.string().required().min(3).max(3).example('EUR'),
    target: joi.string().required().min(3).max(3).example('GBP')
  })
  .unknown()
  .required();

const defaults = {
  timeout: 1000 // 1 sec
};

const exchange = async (pair, options = {}) => {
  options = Object.assign({}, defaults, options);
  const {source, target} = joi.attempt(pair, schema);

  const {requestApi = api, parser = xmlParser} = options;

  const exchange = new Exchange(requestApi, parser, options);
  const rate = await exchange.convert({source, target});
  return {source, target, rate};
};

module.exports = exchange;

exchange.js

'use strict';

const URL = 'https://www.ecb.europa.eu/stats/eurofxref/eurofxref-daily.xml';

class Exchange {
  constructor(api, parser, options = {}) {
    this.api = api;
    this.options = options;
    this.parser = parser;
  }

  async convert({source, target}) {
    if (!this.xml) {
      await this.fetch();
      this.euroToAll = this.parser(this.xml);
    }
    const euroToSource = this.euroToAll[source];
    const euroToTarget = this.euroToAll[target];
    return exchange(euroToSource, euroToTarget);
  }

  async fetch() {
    const response = await this.api.fetch(URL, this.options);
    this.xml = response.body || '';
  }
}

function exchange(from, to) {
  return round(parseFloat(to) / parseFloat(from));
}

function round(result, digits = 4) {
  return Math.round(result * (10 ** digits)) / (10 ** digits);
}

module.exports = Exchange;

parse-xml.js

'use strict';

const xmldoc = require('xmldoc');
const debug = require('debug')('exchange-rate:parse');

const currencies = require('./currencies');

const parse = xml => {
  const doc = new xmldoc.XmlDocument(xml);
  const cube = doc.childNamed('Cube').childNamed('Cube');

  const rates = currencies.reduce(
    (accumulator, currency) => {
      const exchange = cube.childWithAttribute('currency', currency);
      if (exchange) {
        const {rate} = exchange.attr;
        accumulator[currency] = rate;
      } else {
        debug(`Node not found for currency: ${currency}`);
      }
      return accumulator;
    },
    {}
  );
  // Add EUR rate to make it consistent
  rates.EUR = '1.0';
  return rates;
};

module.exports = parse;

api.js

'use strict';

const got = require('got');

module.exports = {
  async fetch(url, options = {}) {
    return got(url, options);
  }
};

test.js

import test from 'ava';
import createExchange from '.';

const xml = require('./fake-response');

test('return exchange rate for valid currency pair', async t => {
  const mockParser = _ => ({
    EUR: 1.0,
    INR: 78.388,
    GBP: 0.875,
    USD: 1.1632
  });
  const mockApi = {
    async fetch(_) {
      return Promise.resolve(xml);
    }
  };
  const options = {
    parser: mockParser,
    requestApi: mockApi
  };
  const exchange = createExchange(options);

  let source = 'EUR';
  let target = 'INR';
  let expected = {
    source,
    target,
    rate: 78.388
  };
  t.deepEqual(await exchange.convert({source, target}, options), expected);

  source = 'USD';
  target = 'INR';
  expected = {
    source,
    target,
    rate: 67.39
  };
  t.deepEqual(await exchange.convert({source, target}, options), expected);

  source = 'INR';
  target = 'USD';
  expected = {
    source,
    target,
    rate: 0.0148
  };
  t.deepEqual(await exchange.convert({source, target}, options), expected);
});

test('throw on invalid input', async t => {
  const mockParser = _ => ({
    EUR: 1.0,
    INR: 79.1505,
    USD: 1.1675
  });
  const mockApi = {
    async fetch(_) {
      return Promise.resolve(xml);
    }
  };
  const options = {
    parser: mockParser,
    requestApi: mockApi
  };
  const exchange = createExchange(options);

  try {
    await exchange.convert('foo');
    t.fail();
  } catch (err) {
    t.pass();
  }

  try {
    await exchange.convert(NaN);
    t.fail();
  } catch (err) {
    t.pass();
  }

  try {
    await exchange.convert(Infinity);
    t.fail();
  } catch (err) {
    t.pass();
  }

  try {
    await exchange.convert({source: 'EUR', target: 'GB'});
    t.fail();
  } catch (err) {
    t.pass();
  }
});

Client code

const createExchange = require('exchange-rate');
const exchange = createExchange();
const {source, target, rate} = await exchange.convert({source: 'EUR', target: 'GBP'});

Questions:

1. What if in the future we need to add different providers with different representations? How can I make it more flexible and keep the core logic decoupled?
2. I am also curious to know if the design of the API from the client perspective is good or it can be improved.
3. In NodeJs we can define dependency via require but I found it difficult to mock them for testing so, I have tried at couple of places to pass dependencies via arguments; is this fine?

Answer 1

What if in future we need to add different providers with different representation? How can I make it more flexible and keep the core logic decoupled?

It seems like the parser in parse-xml.js is tightly coupled to the schema of the XML document at the URL stored in the Exchange class. Suppose there was a need to parse a different XML document (e.g. this list of exchange rates from the Bank of Israel). There are no <Cube> elements in that document - instead there are <CURRENCY> elements. Thus, the code within the parse function would not find the correct rates.

One could have options in the Exchange class of which URL and parser function to use. Another approach might be to have subclasses that contain a specific URL and parsing functions. Or the parser function could look at the schema of the XML and have branched functionality in various cases.

I am also curious to know if the design of the API from the client perspective is good or it can be improved.

If by API you mean the component in api.js then I would say it looks very simple with the one method, but seems to be all that is needed.

If by API you mean the methods of the Exchange class then I would say the convert() method seems simple enough to use. One could also use individual parameters for the source and target currencies instead of a single object that is destructured. The destructuring could also be used to provide the defaults for the requestApi and parser options (as illustrated in this example in the MDN documentation on destructuring.

So the function exchange

const exchange = async (pair, options = {}) => {
  options = Object.assign({}, defaults, options);
  const {source, target} = joi.attempt(pair, schema);

  const {requestApi = api, parser = xmlParser} = options;

  const exchange = new Exchange(requestApi, parser, options);
  const rate = await exchange.convert({source, target});
  return {source, target, rate};
};

Could be updated like this:

const exchange = async (pair, {requestApi = api, parser = xmlParser, options = defaults} = {}) => {
  const {source, target} = joi.attempt(pair, schema);

  const exchange = new Exchange(requestApi, parser, options);
  const rate = await exchange.convert({source, target});
  return {source, target, rate};
};

That way, there is no need to call Object.assign() and the parser and api parameters aren't mixed in with the options object sent to the api requests.

In NodeJs we can define dependency via require but I found it difficult to mock them for testing so, I have tried at couple of places to pass dependencies via arguments, is this fine?

Yes arguments is a good way to implement DI. You could also utilize default parameters, so that in the regular case, the parser can be omitted, but then for other times like testing it can be supplied. The exchange function in index.js is like a factory function and as Greg's answer to this similar question about DI in NodeJS suggests, having the factory function accept the dependecies is a good technique.

For a larger application, a container could be used for Inversion of Control (IoC) (to connect the exchange class with the API and parser) but that may be extra overhead for this application.