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
- Find the duplicate transactions (there can be more than duplicate two entries for a same transaction)
- 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.
- Inside each grouped transactions array, all the transactions should be sorted by time at which they were recorded.
- 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)
})
});