3
\$\begingroup\$

I had an interview task where I had to output customers who live 100km within a particular latitude and longitude and then output them to a file with their username and ID. The customer file looks like the following:

{"latitude": "52.986375", "user_id": 12, "name": "Christina McArdle", "longitude": "-6.043701"}
{"latitude": "51.92893", "user_id": 1, "name": "Alice Cahill", "longitude": "-10.27699"}
{"latitude": "51.8856167", "user_id": 2, "name": "Ian McArdle", "longitude": "-10.4240951"}
{"latitude": "52.3191841", "user_id": 3, "name": "Jack Enright", "longitude": "-8.5072391"}

Here is the code to implement the calculation for the great-circle-distance

/**
 * solution accuired from
 * https://www.movable-type.co.uk/scripts/latlong.html
 */

const greatCircleDistanceCalc = (lat1, lon1, lat2, lon2) => {
  const R = 6371; // Radius of the earth in km
  const dLat = deg2rad(lat2 - lat1); // deg2rad below
  const dLon = deg2rad(lon2 - lon1);
  const a =
    Math.sin(dLat / 2) * Math.sin(dLat / 2) +
    Math.cos(deg2rad(lat1)) *
      Math.cos(deg2rad(lat2)) *
      Math.sin(dLon / 2) *
      Math.sin(dLon / 2);
  const c = 2 * Math.atan2(Math.sqrt(a), Math.sqrt(1 - a));
  const d = R * c; // Distance in km
  return d;
};

const deg2rad = (deg) => {
  return deg * (Math.PI / 180);
};

module.exports = {
    greatCircleDistanceCalc,
}; 

Here is the main app.js where the calculation is used and a customer file is read and a new file is outputted.

const fs = require("fs");
const { greatCircleDistanceCalc } = require("./greatCircleDistanceCalc.js");

const customerFile = fs.readFileSync("./customers.txt", "utf-8"); //Read in file
const customerArr = customerFile.split("\n").map((s) => JSON.parse(s)); //Convert file into array of objects

const dublinOffice = {
  latitude: "53.339428",
  longitude: "-6.257664",
};

const invitedArr = [];

//Sort data in ascending order
const sortedCustomers = customerArr.sort((a, b) => {
  return a.user_id - b.user_id;
});

const closestCustomers = (sortedCustomers, arr) => {
  for ({ latitude, longitude, user_id, name } of sortedCustomers) {
    if (
      greatCircleDistanceCalc(
        dublinOffice.latitude,
        dublinOffice.longitude,
        latitude,
        longitude
      ) <= 100
    ) {
      invitedArr.push(`${name}:${user_id}`);
    }
  }
  writeInvitedCustomer(arr);
};

const writeInvitedCustomer = (arr) => {
  const writeStream = fs.createWriteStream("Output.txt");
  const pathName = writeStream.path;

  arr.forEach((value) => writeStream.write(`${value}\n`));
  writeStream.on("finish", () => {
    console.log(`wrote all the array data to file ${pathName}`);
  });

  writeStream.on("error", (err) => {
    console.error(`There is an error writing the file ${pathName} => ${err}`);
  });

  writeStream.end();
};

closestCustomers(sortedCustomers, invitedArr);

Overall I am happy with the solution but I think the only thing is I am not sure how to write a test for this.

As always any feedback and suggestions are much welcome.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ You 'got away' with it in this instance, but I would advise that you explicitly convert string representations of numbers to numbers if you're to do mathematics with them. Consider what would happen if it had been like 'lat2 + lat1' and not 'lat2 - lat1', you would have concatenated two strings. \$\endgroup\$ Oct 19 '20 at 22:20
2
\$\begingroup\$

Generally I find the code very unstructured making reading it a bit confusing:

First you execute some code outside of a function to read the data from a file, then you define a constant (dublinOffice), then you define a variable (invitedArr), but don't use it in the next block of code, then you sort the data, then you define two functions, and finally execute one of the functions.

Further more the function closestCustomers both gets invitedArr passed an argument (and uses it as such) and uses it directly as a global variable.

Personally I'd have the main code calling functions first (after defining constants) and then define all functions at the end of the source code using function so that they are hoisted. Other people may define the functions first and then use them, but that is a matter of taste, as long it's not all mixed up.

More, smaller functions would also be helpful. Reading the data should be in its own function and closestCustomers does too much (filtering, converting the objects to strings, and calling writeInvitedCustomer). This would also make testing easier.

Using the array methods filter() and map() would help the code be more readable. Personally, I'd start with something like:

const inputFileName = "customers.txt";
const outputFileName = "output.txt";

const dublinOffice = {
  latitude: "53.339428",
  longitude: "-6.257664",
};
const maxDistance = 100;

const invitedCustomers = readCustomers(inputFileName)
  .filter(withinKms(dublinOffice, maxDistance ))
  .sort(userIdComparator)
  .map(formatCustomerForOutput); // `map` may be considered to be moved inside `writeInvitedCustomers`

writeInvitedCustomers(invitedCustomers, outputFileName);


function readCustomers(filename) {
  // ...
}

function withinKms(location, kms) {
   return (customer) => greatCircleDistanceCalc(customer.latitude, customer.longitude, location.latitude, location.longitude) <= kms;
}

function userIdComparator({user_id: userId1}, {user_id: userId2}) {
   return userId1 - userId2;
}

function formatCustomerForOutput({name, user_id}) {
   return `${name}:${user_id}`;
}

// etc.

One other point: I'm not familiar with node's WriteStream, but it looks wrong for me to assign the event handlers after using it by calling its write method. Can't write cause an error?

NB: This is probably not your choice, if this was an interview task, but I find the file format for customers.txt (multiple JOSN objects in one file) strange. A straight forward JSON array would make more sense.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.