Nodejs Interview - API calls and exchange rate

Question

I did an initial technical phone screening with this company which went ok, they then sent me a programming exercise to complete and send back to them.

---

The goal of the exercise:

-take input as a command line arguments (2017年06月03日 USD 100 CAD)

-use the input to make a call to an API (https://api.exchangeratesapi.io to get exchange rate USD,100ドル)

-print the output as a JSON object //will be USD 100ドル -> CAD $x

-complete some specified unit tests and integration tests

-use a linter to validate the JS is clean

---

The program is able to take in arguments, call the API, get back the data, compute the data, and print the correct data.

So I am suspecting that I may not be following "best practices" for development in node.js.

If anyone can look through the code and let me know of any red flags, such as outdated JS stuff, improper methodologies, or just anything bad you can see that would be very helpful!!!

https://github.com/mkbru/CiteRightChallenge

app.js

const errorChecks = require("./utils/errorChecks");
const getExchangeRate = require("./utils/getExchangeRate");
const args = process.argv;
const date = args[2];
const bCurrency = args[3];
const bAmount = parseInt(args[4]);
const cCurrency = args[5];
//user input validation
errorChecks(process.argv);
getExchangeRate(date, bCurrency, bAmount, cCurrency).then(output =>{
 console.log(output);
});

errorchecks.js

const isCurrency = require('./isCurrency')
function errorChecks(args){
 const date = args[2];
 const bCurrency = args[3];
 const bAmount = parseInt(args[4]);
 const cCurrency = args[5];
 if(args.length != 6){
 console.log("Usage Error: Incorrect amount of arguments");
 process.exit(1);
 }
 if(!(/^\d{4}[\-.]\d{1,2}[\-.]\d{1,2}$/.test(date))){
 console.log("Usage Error: "+ date +" is not a date");
 process.exit(1);
 }
 isCurrency(bCurrency).then(output => {
 if(!output) {
 console.log("Usage Error: " + cCurrency + " is not a currency.");
 process.exit(1);
 }
 });
 if(isNaN(bAmount)){
 console.log("Usage Error: " + bAmount + " is not a number.");
 process.exit(1);
 }
 isCurrency(cCurrency).then(output => {
 if(!output){
 console.log("Usage Error: " + cCurrency + " is not a currency.");
 process.exit(1);
 }
 });
}
module.exports = errorChecks;

getExchangeRate.js

const request = require('request-promise');
function getExchangeRate(date, bCurrency, bAmount, cCurrency){
 const options = {
 method: 'GET',
 uri: 'https://api.exchangeratesapi.io/'+date+'?base='+bCurrency,
 json: true
 }
 return request(options).
 then(function (response){
 exRate = response.rates[cCurrency]
 cAmount = (exRate) * bAmount; //conversion rate * base amount
 const output = {
 date: date,
 base_currency: bCurrency,
 base_amount: bAmount,
 conversion_currency: cCurrency,
 conversion_amount: Math.round(cAmount * 100) / 100
 }
 return output;
 }).
 catch(function (err){
 console.log(err)
 })
}
module.exports = getExchangeRate;

getCurrencies.js

const request = require('request-promise');
function getCurrencies(){
 const options = {
 method: 'GET',
 uri: 'https://api.exchangeratesapi.io/latest',
 json: true
 }
 return request(options).
 then(function (response){
 let currencies = [];
 for(let k in response.rates) currencies.push(k);
 return currencies;
 }).
 catch(function (err){
 console.log(err)
 })
}
module.exports = getCurrencies;

isCurrency.js

const request = require('request-promise');
const getCurrencies = require('./getCurrencies')
function isCurrency(currency){
 return getCurrencies().then(currencies => {
 let isCurrency = currencies.includes(currency);
 return isCurrency;
 });
}
module.exports = isCurrency;

index.test.js

const chai = require("chai");
const chaiHttp = require("chai-http");
const expect = require("chai").expect;
const getExchangeRate = require("../utils/getExchangeRate");
chai.use(chaiHttp);
describe("First test", () => {
 it("Should assert true to be true", () => {
 expect(true).to.be.true;
 });
});
describe("Test if API is running",() =>{
 it("Should assert to true", (done) => {
 chai.request("https://api.exchangeratesapi.io")
 .get('/latest')
 .end((err,res) => {
 expect(res).to.have.status(200);
 done();
 });
 });
});
describe("getExchangeRate()_unit-test-1", () =>{
 it("should assert true if obj1 === obj2", (done) => {
 const date = "2011-06-03";
 const base_currency = "USD";
 const base_amount = 100;
 const conversion_currency = "CAD";
 const obj1 = {
 date: "2011-06-03",
 base_currency: "USD",
 base_amount: 100,
 conversion_currency: "CAD",
 conversion_amount: 97.85,
 };
 getExchangeRate(date,base_currency,base_amount,conversion_currency).
 then(obj2 => {
 expect(obj1).to.eql(obj2);
 done();
 }).
 catch(error => {
 done(error);
 });
 });
});
describe("getExchangeRate()_unit-test-2", () =>{
 it("should assert true if obj1 === obj2", (done) => {
 const date = "2007-07-12";
 const base_currency = "GBP";
 const base_amount = 303;
 const conversion_currency = "SEK";
 const obj1 = {
 date: "2007-07-12",
 base_currency: "GBP",
 base_amount: 303,
 conversion_currency: "SEK",
 conversion_amount: 4085.015
 };
 getExchangeRate(date,base_currency,base_amount,conversion_currency).
 then(obj2 => {
 expect(obj1).to.eql(obj2);
 done();
 }).
 catch(error => {
 done(error);
 });
 });
});
describe("getExchangeRate()_unit-test-3", () =>{
 it("should assert true if obj1 === obj2", (done) => {
 const date = "2004-08-07";
 const base_currency = "EUR";
 const base_amount = 5;
 const conversion_currency = "PLN";
 const obj1 = {
 date: "2004-08-07",
 base_currency: "EUR",
 base_amount: 5,
 conversion_currency: "PLN",
 conversion_amount: 22.01,
 };
 getExchangeRate(date,base_currency,base_amount,conversion_currency).
 then(obj2 => {
 expect(obj1).to.eql(obj2);
 done();
 }).
 catch(error => {
 done(error);
 });
 });
});
describe("getExchangeRate()_unit-test-4", () =>{
 it("should assert true if obj1 === obj2", (done) => {
 const date = "2017-02-09";
 const base_currency = "ZAR";
 const base_amount = 132;
 const conversion_currency = "TRY";
 const obj1 = {
 date: "2017-02-09",
 base_currency: "ZAR",
 base_amount: 132,
 conversion_currency: "TRY",
 conversion_amount: 36.3528,
 };
 getExchangeRate(date,base_currency,base_amount,conversion_currency).
 then(obj2 => {
 expect(obj1).to.eql(obj2);
 done();
 }).
 catch(error => {
 done(error);
 });
 });
});

Answer 1

Here's 5 things I noticed right away:

1. errorChecks(process.argv) is probably best before you try to parse the arguments. Some of that parsing is going to fail before getting to your nice error messages.

2. Edit your tests-- remove boilerplate ones and give the remaining ones good names.

3. You have some temporary variables where you don't need them: let isCurrency = currencies.includes(currency); return isCurrency Can just be return currencies.includes(currency). (And any let that is not modified is better as a const.)

4. This logic seems flawed unless you're restricted to CAD and US$: Math.round(cAmount * 100) / 100

5. I think: let currencies = []; for(let k in response.rates) currencies.push(k); return currencies; can simply be return response.rates.