2
\$\begingroup\$

I had given a coding test for a job which failed. It had two problems and one of them is shared in this question along with my solution and other I already posted at this link.

Problem: Find duplicate transactions.

Description: Usually, due to any technical mistake, a single transaction is recorded twice in the database. Assume the time difference between duplicate transactions is always less than 60 seconds. Duplicate transactions has same values for sourceAccount, targetAccount, category and amount.

This is how a typical transaction looks like:

{
  id: 123,
  sourceAccount: 'my_account',
  targetAccount: 'grocery_shop',
  amount: -30,
  category: 'groceries',
  time: '2018-03-12T12:34:00Z'
}

Negative value for amount means amount has been spent in that transaction.

Solution Requirements

  1. Find the duplicate transactions (there can be more than duplicate two entries for a same transaction)
  2. Group them in arrays. Each array has all the duplicates of a transaction including first transaction record as well. (Let's call it grouped transactions array). Final output will be an array of these grouped transactions array.
  3. Inside each grouped transactions array, all the transactions should be sorted by time at which they were recorded.
  4. Final array should contain the grouped transactions arrays sorted by time of their first elements.

General Requirements

Here is what they said they are looking for in my solution:

This is a coding challenge which tests your coding abilities and to make sure you can present us with well written, well tested and not over-engineered code. We're looking for a well structured, tested, simple solution. As mentioned before the engineering teams work in a TDD environment and code is driven by the testing methodology as we are deploying code on a daily basis. It's a very collaborative environment so there is a lot of pair and mob programming happening which is why the code that is written needs to be able to be understood by others in your team.

My Solution:

let moment = require('moment')

exports.findDuplicateTransactions = function (transactions = []) {
  
  let duplicates = []
  
  transactions.forEach((transaction, index) => {
    for (let i=0; i<transactions.length; i++) {
      if (index !== i) {
        if (isDuplicateTransactions(transaction, transactions[i])) {
          if (duplicates.indexOf(transactions[i]) === -1) {
            duplicates.push(transactions[i])
          }
        }
      }
    }
  })
  
  let duplicateTransactionsGroups = groupBy(duplicates, function(item) {
    return [item.sourceAccount, item.targetAccount,
           item.amount, item.category];
  });
  
  let transactionsGroupsMembersSorted = duplicateTransactionsGroups.map(group => {
    return group.slice().sort((obj1, obj2) => {
      return new Date(obj1.time) - new Date(obj2.time);
    })
  });
  
  let transactionsGroupsSorted = transactionsGroupsMembersSorted.slice().sort((obj1, obj2) => {
    return new Date(obj1[0].time) - new Date(obj2[0].time)
  })
  
  return transactionsGroupsSorted
}

const isDuplicateTransactions = function (transaction1, transaction2) {
  let date1 = moment(transaction1.time)
  let date2 = moment(transaction2.time)
  let difference = Math.abs(date1.diff(date2, 'seconds'))
  if (transaction1.sourceAccount === transaction2.sourceAccount &&
      transaction1.targetAccount === transaction2.targetAccount &&
      transaction1.category === transaction2.category &&
      transaction1.amount === transaction2.amount &&
      difference < 60
     ) {
      return true
  }
  return false
}

const groupBy = function ( list , f ){
  var groups = {};
  list.forEach( function( item )
  {
    var group = JSON.stringify( f(item) );
    groups[group] = groups[group] || [];
    groups[group].push( item );  
  });
  return Object.keys(groups).map( function( group )
  {
    return groups[group]; 
  })
}

exports.groupBy = groupBy
exports.isDuplicateTransactions = isDuplicateTransactions

All the unit tests for this function pass. The summarized feedback of my both solutions was that the code is inefficient.

Here is the detailed feedback to me by those who reviewed my submitted code:

  • Solutions are overall inefficient. Naming convention is good the idea behind some unitary functions is well intended but badly executed. (negative feedback)
  • Easy to Maintain, (positive feedback)
  • Easy to Read, (positive feedback)
  • Advanced Grasp of Language (positive feedback)
  • Inefficient Solution (negative feedback)
  • There is actually no need to have two loops for this exercise. But it's well structured and readable. Since it's doing two different things a better result of this approach would have been to have two different functions. (General Feedback)

I am understanding part of the feedback. It is overall feedback to both coding problems and I have only presented one of them here. I am not sure to which one the feedback applies. I have published the first one.

Please let me know what else can be improved in my code and how can I make it efficient. Please give me your complete feedback regarding efficiency, performance etc.

Thank you.

Here are my test cases:

const assert = require("chai").assert;
const { findDuplicateTransactions, isDuplicateTransactions, groupBy } = require('./../transactions2')

describe("findDuplicateTransactions()", function () {
  it("returns empty array if there are no transactions", function () {
    assert.deepEqual(findDuplicateTransactions([]), []);
  });

  it("returns empty array if there is only one transaction", function () {
    assert.deepEqual(findDuplicateTransactions([{
      id: 3,
      sourceAccount: 'A',
      targetAccount: 'B',
      amount: 150,
      category: 'other',
      time: '2018-03-02T10:34:30.000Z'
    }]), []);
  });

  it("returns empty array if there are no duplicates", function () {
    assert.deepEqual(findDuplicateTransactions([{
      id: 3,
      sourceAccount: 'A',
      targetAccount: 'B',
      amount: 150,
      category: 'other',
      time: '2018-03-02T10:34:30.000Z'
    },
    {
      id: 1,
      sourceAccount: 'A',
      targetAccount: 'C',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:00.000Z'
    }]), []);
  });

  it("returns an array of length 1 if found one group only", function () {
    assert.lengthOf(findDuplicateTransactions([
      {
        id: 3,
        sourceAccount: 'A',
        targetAccount: 'B',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:30.000Z'
      },
      {
        id: 1,
        sourceAccount: 'A',
        targetAccount: 'B',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:01.000Z'
      }
    ]), 1);
  });

  it("returns an array of sorted groups of duplicate transactions", function () {
    let sourceList = [
      {
        id: 3,
        sourceAccount: 'A',
        targetAccount: 'B',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:34:30.000Z'
      },
      {
        id: 1,
        sourceAccount: 'A',
        targetAccount: 'B',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:01.000Z'
      },
      {
        id: 6,
        sourceAccount: 'A',
        targetAccount: 'B',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:05.000Z'
      },
      {
        id: 19,
        sourceAccount: 'C',
        targetAccount: 'D',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:00.000Z'
      },
      {
        id: 18,
        sourceAccount: 'C',
        targetAccount: 'D',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:34:10.000Z'
      },
      {
        id: 17,
        sourceAccount: 'C',
        targetAccount: 'D',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:50.000Z'
      },
      {
        id: 4,
        sourceAccount: 'A',
        targetAccount: 'B',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:36:00.000Z'
      },
      {
        id: 2,
        sourceAccount: 'A',
        targetAccount: 'B',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:50.000Z'
      },
      {
        id: 5,
        sourceAccount: 'A',
        targetAccount: 'C',
        amount: 250,
        category: 'other',
        time: '2018-03-02T10:33:00.000Z'
      }
    ]
    let targetList = [[{
      id: 19,
      sourceAccount: 'C',
      targetAccount: 'D',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:00.000Z'
    },
    {
      id: 17,
      sourceAccount: 'C',
      targetAccount: 'D',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:50.000Z'
    },
    {
      id: 18,
      sourceAccount: 'C',
      targetAccount: 'D',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:34:10.000Z'
    }],
    [{
      id: 1,
      sourceAccount: 'A',
      targetAccount: 'B',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:01.000Z'
    },
    {
      id: 6,
      sourceAccount: 'A',
      targetAccount: 'B',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:05.000Z'
    },
    {
      id: 2,
      sourceAccount: 'A',
      targetAccount: 'B',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:50.000Z'
    },
    {
      id: 3,
      sourceAccount: 'A',
      targetAccount: 'B',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:34:30.000Z'
    }]]
    assert.deepEqual(findDuplicateTransactions(sourceList), targetList);
  });
});

describe("isDuplicateTransactions()", function () {
  it("returns true if transactions are duplicate", function () {
    assert.equal(isDuplicateTransactions({
      id: 19,
      sourceAccount: 'C',
      targetAccount: 'D',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:05.000Z'
    },
      {
        id: 18,
        sourceAccount: 'C',
        targetAccount: 'D',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:15.000Z'
      }), true)
  })

  it("returns false if transactions are not duplicate", function () {
    assert.equal(isDuplicateTransactions({
      id: 19,
      sourceAccount: 'C',
      targetAccount: 'D',
      amount: 100,
      category: 'other',
      time: '2018-03-02T10:33:05.000Z'
    },
      {
        id: 18,
        sourceAccount: 'C',
        targetAccount: 'D',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:15.000Z'
      }), false)
  })

  it("returns false if transactions time difference is greater than 1 minute", function () {
    assert.equal(isDuplicateTransactions({
      id: 19,
      sourceAccount: 'C',
      targetAccount: 'D',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:36:05.000Z'
    },
      {
        id: 18,
        sourceAccount: 'C',
        targetAccount: 'D',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:15.000Z'
      }), false)
  })
});

describe("groupBy()", function () {
  it("return empty array if given empty array", function () {
    assert.deepEqual(groupBy([], function (item) {
      return [item.sourceAccount, item.targetAccount,
      item.amount, item.category];
    }), [])
  })

  it("returns items of array grouped in arrays by given keys", function () {
    let sourceList = [{
      id: 6,
      sourceAccount: 'A',
      targetAccount: 'B',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:05.000Z'
    },
    {
      id: 1,
      sourceAccount: 'A',
      targetAccount: 'B',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:00.000Z'
    },
    {
      id: 18,
      sourceAccount: 'C',
      targetAccount: 'D',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:50.000Z'
    },
    {
      id: 19,
      sourceAccount: 'C',
      targetAccount: 'D',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:05.000Z'
    },
    {
      id: 17,
      sourceAccount: 'C',
      targetAccount: 'D',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:34:10.000Z'
    }]

    const targetList = [[{
      id: 6,
      sourceAccount: 'A',
      targetAccount: 'B',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:05.000Z'
    },
    {
      id: 1,
      sourceAccount: 'A',
      targetAccount: 'B',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:00.000Z'
    }],
    [{
      id: 18,
      sourceAccount: 'C',
      targetAccount: 'D',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:50.000Z'
    },
    {
      id: 19,
      sourceAccount: 'C',
      targetAccount: 'D',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:05.000Z'
    },
    {
      id: 17,
      sourceAccount: 'C',
      targetAccount: 'D',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:34:10.000Z'
    }]]
    assert.deepEqual(groupBy(sourceList, function (item) {
      return [item.sourceAccount, item.targetAccount,
      item.amount, item.category];
    }), targetList)
  })
});
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

General code style

  • When you aren't going to reassign a variable, you should declare it with const. Only use let when you have to reassign. Never use var, since it has too many problems to be worth using.

  • Unless you need a full function to capture the this context or a function declaration, you might consider using arrow functions by default when you need a function expression - arrow functions are more concise not only in their lack of the function keyword, but they also permit implicit return, which lets you omit the {} function block and return. It's great for short callbacks, eg:

    group.slice().sort(
      (obj1, obj2) => new Date(obj1.time) - new Date(obj2.time)
    )
    

Array.prototype.includes When you want to check whether an element exists in an array, it'd be more appropriate to use .includes(item) rather than indexOf(item) === -1 - it's easier to read.

Object values In groupBy, when you need to find the values of an object, you can use Object.values. That is, this:

return Object.keys(groups).map( function( group )
{
  return groups[group]; 
})

can turn into

return Object.values(groups);

Comments Your original code has no comments, and it's not quite self-documenting enough IMO. When the intent of a particular section isn't blindingly obvious at a glance, don't be afraid to add comments liberally. When dealing with complicated data structures, giving examples of what a particular section of code results in can make things significantly clearer to a casual reader.

Efficiency The main efficiency issue I see is the nested loops here:

transactions.forEach((transaction, index) => {
  for (let i = 0; i < transactions.length; i++) {
    if (index !== i) {
      if (isDuplicateTransactions(transaction, transactions[i])) {
        if (duplicates.indexOf(transactions[i]) === -1) {
          duplicates.push(transactions[i])
        }
      }
    }
  }
})

This compares each transaction to every other transaction, which is O(n ^ 2). Then, after comparison, you call duplicates.indexOf(transactions[i]) === -1, and indexOf is O(n). Put it together, and it's not that great.

One way to reduce complexity would be to use a Set instead of an array for the duplicates. Sets can be looked up in O(1) time, rather than O(n) time for arrays.

Another way to reduce complexity would be to group the transactions while iterating. Instead of comparing each element to each other element, construct a mostly unique key first, composed of the identical properties. For example:

{
  id: 3,
  sourceAccount: 'A',
  targetAccount: 'B',
  amount: 100,
  category: 'eating_out',
  time: '2018-03-02T10:33:30.000Z'
},

can turn into a key of A-B-eating_out-100. When this element is found, put this key onto the object. When iterating over an element, if a duplicate key is found on the object, compare the times of each element in the object to see if it's truly a duplicate, or if they're separated by more than 60 seconds and aren't duplicates. This reduces the nested loop logic's complexity because rather than comparing against every other transaction, you're only comparing against likely duplicate transactions. This could reduce the nested loop logic's complexity from O(n ^ 2) to around O(n).

It's unfortunate that there's no unique identifier for transactions; that'd make things so much easier. If this was a real-world problem, I'd work on changing the code that generates the array so that duplicate transactions appear with the same ID (or some other unique identifier for the same transaction, like a GUID generated and passed along before the problematic code is encountered).

Related to the above, you look have a bug in that your current duplicateTransactionsGroups is generated by a groupBy on the duplicates array, checking item.sourceAccount, item.targetAccount, item.amount, item.category without checking for time. If there are multiple duplicate entries (say, two on Monday and two on Tuesday) with the same attributes, they'll be grouped together, even though they shouldn't be.

Another thing that will improve efficiency would be to group the transactions for the output at the same time that you're checking for duplicates, instead of doing groupBy later after the array of duplicates is constructed.

You're also importing Moment for the sole purpose of checking whether the difference between two date strings is more or less than 60 seconds. This is trivial and faster to accomplish in vanilla JS; just call new Date on the time strings and compare the differences in their timestamps.

One way to improve the date sorting would be to sort the entire input array by date beforehand. Then, the resulting groups will come out sorted naturally (no further sorting needed) since they'll be processed sequentially, and you won't have to worry about if a transaction at 200s is a duplicate of a transaction already seen at 100s because the connector at 150s hasn't been seen yet. This one insight is a huge improvement to the overall algorithm IMO.

Putting these recommendations together, and you'll get computational complexity of O(n log n). Because the output must be sorted by time, and such sorting requires O(n log n) complexity (or thereabouts, for this sort of input), further optimization would be quite difficult for not much gain. Overall, the code could look like:

const getTransactionKey = ({
  sourceAccount,
  targetAccount,
  category,
  amount
}) => `${sourceAccount}-${targetAccount}${category}${amount}`;

const findDuplicateTransactions = (transactions = []) => {
  transactions.sort((a, b) => new Date(a.time) - new Date(b.time));
  const transactionsByKey = {};
  for (const transaction of transactions) {
    const key = getTransactionKey(transaction);
    transactionsByKey[key] = transactionsByKey[key] || [];
    transactionsByKey[key].push(transaction);
  }
  
  // Separate each transactionsByKey[key] array into arrays of definite duplicates
  // and combine all such arrays of definite duplicates into a single array
  const allTransactionGroups = Object.values(transactionsByKey).flatMap(groupDuplicates);
  
  const duplicateTransactionGroups = allTransactionGroups.filter(subarr => subarr.length >= 2);
  
  return duplicateTransactionGroups;
};

/**
 * Separate each transactionsByKey[key] array into arrays of definite duplicates, eg:
 * [{ source: 'A' ... }, { source: 'B' ... }, { source: 'B' ... }]
 * to
 * [[{ source: 'A' ... }], [{ source: 'B' ... }, { source: 'B' ... }]]
 */
const groupDuplicates = (similarTransactions) => {
  const duplicateGroups = [];
  for (const transaction of similarTransactions) {
    // Find the first subarray in duplicateGroups whose time matches, and push to that subarray
    // If no match, create a new subarray
    const foundGroup = duplicateGroups.find(
      subarr => isDuplicateTime(subarr[subarr.length - 1], transaction)
    );
    if (foundGroup) {
      foundGroup.push(transaction)
    } else {
      duplicateGroups.push([transaction]);
    }
  }
  return duplicateGroups;
};
  
const isDuplicateTime = (transaction1, transaction2) => (
  Math.abs(new Date(transaction1.time) - new Date(transaction2.time)) < 60_000
);

Live snippet:

const getTransactionKey = ({
  sourceAccount,
  targetAccount,
  category,
  amount
}) => `${sourceAccount}-${targetAccount}${category}${amount}`;

const findDuplicateTransactions = (transactions = []) => {
  transactions.sort((a, b) => new Date(a.time) - new Date(b.time));
  const transactionsByKey = {};
  for (const transaction of transactions) {
    const key = getTransactionKey(transaction);
    transactionsByKey[key] = transactionsByKey[key] || [];
    transactionsByKey[key].push(transaction);
  }
  
  // Separate each transactionsByKey[key] array into arrays of definite duplicates
  // and combine all such arrays of definite duplicates into a single array
  const allTransactionGroups = Object.values(transactionsByKey).flatMap(groupDuplicates);
  
  const duplicateTransactionGroups = allTransactionGroups.filter(subarr => subarr.length >= 2);
  
  return duplicateTransactionGroups;
};

/**
 * Separate each transactionsByKey[key] array into arrays of definite duplicates, eg:
 * [{ source: 'A' ... }, { source: 'B' ... }, { source: 'B' ... }]
 * to
 * [[{ source: 'A' ... }], [{ source: 'B' ... }, { source: 'B' ... }]]
 */
const groupDuplicates = (similarTransactions) => {
  const duplicateGroups = [];
  for (const transaction of similarTransactions) {
    // Find the first subarray in duplicateGroups whose time matches, and push to that subarray
    // If no match, create a new subarray
    const foundGroup = duplicateGroups.find(
      subarr => isDuplicateTime(subarr[subarr.length - 1], transaction)
    );
    if (foundGroup) {
      foundGroup.push(transaction)
    } else {
      duplicateGroups.push([transaction]);
    }
  }
  return duplicateGroups;
};
  
const isDuplicateTime = (transaction1, transaction2) => (
  Math.abs(new Date(transaction1.time) - new Date(transaction2.time)) < 60_000
);









// TESTING
const assert = {
  deepEqual(a, b) {
    if (JSON.stringify(a) !== JSON.stringify(b)) {
      throw new Error('Failed');
    }
  },
  lengthOf(a, len) {
    if (a.length !== len) {
      throw new Error('Failed');
    }
  }
}
const it = (str, fn) => {
  console.log(str);
  fn();
};

  it("returns empty array if there are no transactions", function () {
    assert.deepEqual(findDuplicateTransactions([]), []);
  });

  it("returns empty array if there is only one transaction", function () {
    assert.deepEqual(findDuplicateTransactions([{
      id: 3,
      sourceAccount: 'A',
      targetAccount: 'B',
      amount: 150,
      category: 'other',
      time: '2018-03-02T10:34:30.000Z'
    }]), []);
  });

  it("returns empty array if there are no duplicates", function () {
    assert.deepEqual(findDuplicateTransactions([{
      id: 3,
      sourceAccount: 'A',
      targetAccount: 'B',
      amount: 150,
      category: 'other',
      time: '2018-03-02T10:34:30.000Z'
    },
    {
      id: 1,
      sourceAccount: 'A',
      targetAccount: 'C',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:00.000Z'
    }]), []);
  });

  it("returns an array of length 1 if found one group only", function () {
    assert.lengthOf(findDuplicateTransactions([
      {
        id: 3,
        sourceAccount: 'A',
        targetAccount: 'B',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:30.000Z'
      },
      {
        id: 1,
        sourceAccount: 'A',
        targetAccount: 'B',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:01.000Z'
      }
    ]), 1);
  });

  it("returns an array of sorted groups of duplicate transactions", function () {
    let sourceList = [
      {
        id: 3,
        sourceAccount: 'A',
        targetAccount: 'B',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:34:30.000Z'
      },
      {
        id: 1,
        sourceAccount: 'A',
        targetAccount: 'B',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:01.000Z'
      },
      {
        id: 6,
        sourceAccount: 'A',
        targetAccount: 'B',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:05.000Z'
      },
      {
        id: 19,
        sourceAccount: 'C',
        targetAccount: 'D',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:00.000Z'
      },
      {
        id: 18,
        sourceAccount: 'C',
        targetAccount: 'D',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:34:10.000Z'
      },
      {
        id: 17,
        sourceAccount: 'C',
        targetAccount: 'D',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:50.000Z'
      },
      {
        id: 4,
        sourceAccount: 'A',
        targetAccount: 'B',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:36:00.000Z'
      },
      {
        id: 2,
        sourceAccount: 'A',
        targetAccount: 'B',
        amount: 100,
        category: 'eating_out',
        time: '2018-03-02T10:33:50.000Z'
      },
      {
        id: 5,
        sourceAccount: 'A',
        targetAccount: 'C',
        amount: 250,
        category: 'other',
        time: '2018-03-02T10:33:00.000Z'
      }
    ]
    let targetList = [[{
      id: 19,
      sourceAccount: 'C',
      targetAccount: 'D',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:00.000Z'
    },
    {
      id: 17,
      sourceAccount: 'C',
      targetAccount: 'D',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:50.000Z'
    },
    {
      id: 18,
      sourceAccount: 'C',
      targetAccount: 'D',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:34:10.000Z'
    }],
    [{
      id: 1,
      sourceAccount: 'A',
      targetAccount: 'B',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:01.000Z'
    },
    {
      id: 6,
      sourceAccount: 'A',
      targetAccount: 'B',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:05.000Z'
    },
    {
      id: 2,
      sourceAccount: 'A',
      targetAccount: 'B',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:33:50.000Z'
    },
    {
      id: 3,
      sourceAccount: 'A',
      targetAccount: 'B',
      amount: 100,
      category: 'eating_out',
      time: '2018-03-02T10:34:30.000Z'
    }]]
    assert.deepEqual(findDuplicateTransactions(sourceList), targetList);
  });
  console.log('all succeeded');

If needed, you could slightly improve performance by mapping each time string to its timestamp equivalent while sorting to avoid duplicate calls to new Date, but it'd make the code a tiny bit more complicated and a bit harder to understand at a glance. Past a certain point, performance improvements come at the cost of code clarity.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks. This is very detailed and useful review. I will go through it and implement each point and also compare with your given code then. \$\endgroup\$
    – SamFast
    Commented Sep 24, 2020 at 18:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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