In an interview I was asked to solve two JavaScript questions. I thought I did pretty well because I:
- Covered the edge cases
- Wrote comprehensive tests
- Documented the code using jsdoc
The interviewer responded with this exact feedback:
Overly Complex Solution, Strange Coding Conventions, Poorly Structured, Hard to Understand, Overly Complex Solution but the first solution did handle edge cases well.
I'm a little confused by the feedback (why include "Overly Complex" twice and "Hard to Understand", which is basically the same thing) - I would appreciate a second set of eyes and more detailed feedback on why this is "Overly Complex."
English is not my first language; please let me know if anything is unclear.
Q1. Given transactions calculate the balance between start-end time for a specific category.
// Sample transactions, if we say find category:eating transactions between 2020-01-01, 2020-02-25 it should return 400 [{ id:1, amount:100, category:'eating', sourceAccount: 'A', targetAccount: 'B', time: '2020-01-02T00:00:00Z' }, { id:2, amount:210, category:'shopping', sourceAccount: 'A', targetAccount: 'B', time: '2020-01-01T00:00:00Z' }, { id:3, amount:300, category:'eating', sourceAccount: 'A', targetAccount: 'B', time: '2020-02-02T02:02:00Z' }]
// My code
/**
* Transaction object from consumer bank account
* @typedef {Object} Transaction
* @property {number} id
* @property {string} sourceAccount
* @property {string} targetAccount
* @property {number} amount
* @property {string} category
* @property {string} time
*/
/**
* Calculate the balance in a specific category within the specified time period.
* @param {Transaction} [transactions=[]] - Account transactions
* @param {string} category - Target category
* @param {object} startTime - Beginning period (Date object)
* @param {object} endTime - End period (Date object)
* @returns {number} Calculated balance
*/
function getBalanceByCategoryInPeriod(transactions = [], category, startTime, endTime) {
const {
id:1,
amount:100,
category:'eating',
sourceAccount: 'A',
targetAccount: 'B',
time: '2018-03-12T12:33:00Z'
}= transactions
if (!isString(category)) throw new Error('Parameter category is not a string');
if (!isValidDateObject(startTime)) throw new Error('Parameter startTime is not a valid date string');
if (!isValidDateObject(endTime)) throw new Error('Parameter endTime is not a valid date string');
if (endTime <= startTime) throw new Error('End time cannot be lower than start time');
return transactions.reduce((acc, cur) => {
const errors = isValidTransactionObject(cur, transactionSchema);
if (errors.length > 0) {
console.warn(`Transaction properties are not valid, calculating total balance without, ${cur.id}`, errors);
return acc;
}
const targetDate = new Date(cur.time).getTime();
const isBetweenTimeRange = endTime.getTime() > targetDate && targetDate >= startTime.getTime();
if (cur.category === category && isBetweenTimeRange) return acc + cur.amount;
return acc;
}, 0);
}
/*** UTILS ***/
const isString = value => typeof value === 'string';
const isNumber = value => typeof value === 'number' && !isNaN(value);
const isValidDateString = value => /(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d\.\d+)|(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d)|(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d)/.test(value);
const isValidDateObject = value => value instanceof Date && !isNaN(value);
const transactionSchema = {
id: isNumber,
amount: isNumber,
sourceAccount: isString,
targetAccount: isString,
category: isString,
time: isValidDateString
};
const isValidTransactionObject = (object = {}, schema) => {
if (!schema) throw new Error('Schema must be provided to validator function');
return Object.keys(schema)
.filter(key => !schema[key](object[key]))
.map(key => new Error(`${key} is invalid.`));
};
Q2. Given transactions find duplicate transactions (same amount, category, sourceAccount, targetAccount) within one minute period and return them as grouped, grouped arrays should be ordered by first element's id inside of that array. (This one is pretty hard to explain, so just check out input and expected output)
//Sample input [ { id: 3, sourceAccount: 'A', targetAccount: 'B', amount: 100, category: 'eating_out', time: '2018-03-02T10:34:30.000Z' }, { id: 5, sourceAccount: 'A', targetAccount: 'B', amount: 100, category: 'eating_out', time: '2018-03-02T10:33:00.000Z' }, { id: 6, sourceAccount: 'A', targetAccount: 'C', amount: 250, category: 'other', time: '2018-03-02T10:33:05.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: 1, sourceAccount: 'A', targetAccount: 'C', amount: 250, category: 'other', time: '2018-03-02T10:33:00.000Z' } ];//Expected Output [ [ { id: 1, sourceAccount: 'A', targetAccount: 'C', amount: 250, category: 'other', time: '2018-03-02T10:33:00.000Z' }, { id: 6, sourceAccount: 'A', targetAccount: 'C', amount: 250, category: 'other', time: '2018-03-02T10:33:05.000Z' } ], { id: 5, sourceAccount: 'A', targetAccount: 'B', amount: 100, category: 'eating_out', time: '2018-03-02T10:33:00.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' } ] ]
My code:
/**
* Transaction object from consumer bank account
* @typedef {Object} Transaction
* @property {number} id
* @property {string} sourceAccount
* @property {string} targetAccount
* @property {number} amount
* @property {string} category
* @property {string} time
*/
const MUST_MATCH_PROPS = ['sourceAccount', 'targetAccount', 'amount', 'category'];
/**
* Groups sorted transactions array based on conditions
* @param {[Transaction]} arr
* @returns {Array.<Array.<Transaction>>]} - Duplicate Transactions
*/
const groupDuplicateTransactions = arr => {
const subArrays = [];
for (let runner = 1, prevDuplicate = false, arrInd = -1; runner < arr.length; runner++) {
if (isDuplicate(arr[runner], arr[runner - 1])) {
if (!prevDuplicate) arrInd++;
if (!subArrays[arrInd]) subArrays[arrInd] = [];
subArrays[arrInd].push(arr[runner - 1]);
if (runner === arr.length - 1) subArrays[arrInd].push(arr[runner]);
prevDuplicate = true;
} else {
if (prevDuplicate) subArrays[arrInd].push(arr[runner - 1]);
prevDuplicate = false;
}
}
return subArrays;
}
/**
* Finds duplicate transactions in n*log(n)
* @param {[Transaction]} [transactions=[]] - Account transactions
* @returns {[Transaction]} - Duplicate Transactions
*/
const findDuplicateTransactions = (transactions = []) => {
const sortedTransactions = [...transactions] // To ensure immutability. If there is memory constraints I'd prefer mutating
.filter(transaction => isValidTransactionObject(transaction, transactionSchema).length === 0) // Just to be sure objects are valid transaction objects, objects which are not can be ignored while running algorithm but for the sake of cleanness I have decided to filter out beforehands
.sort(sorter);
return groupDuplicateTransactions(sortedTransactions).sort((x, y) => x[0].id - y[0].id);
}
/*** UTILS ***/
const compareFields = (x, y, fields) => fields.every(p => x[p] === y[p]);
const isInOneMinute = (x, y) => Math.abs(new Date(x.time) - new Date(y.time)) < 60000;
const isDuplicate = (x, y) => compareFields(x, y, MUST_MATCH_PROPS) && isInOneMinute(x, y);
const sorter = (a, b) => {
if (a.sourceAccount === b.sourceAccount) {
if (a.targetAccount === b.targetAccount) {
if (a.amount === b.amount) {
if (a.category === b.category) {
return b.time < a.time ? 1 : -1;
}
return b.category < a.category ? 1 : -1;
}
return b.amount < a.amount ? 1 : -1;
}
return b.targetAccount < a.targetAccount ? 1 : -1;
}
return b.sourceAccount < a.sourceAccount ? 1 : -1;
};
const isString = value => typeof value === 'string';
const isNumber = value => typeof value === 'number' && !isNaN(value);
const isValidDateString = value =>
/(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d\.\d+)|(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d)|(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d)/.test(
value
);
const isValidDateObject = value => value instanceof Date && !isNaN(value);
const transactionSchema = {
id: isNumber,
amount: isNumber,
sourceAccount: isString,
targetAccount: isString,
category: isString,
time: isValidDateString
};
const isValidTransactionObject = (object = {}, schema) => {
if (!schema) throw new Error('Schema must be provided to validator function');
return Object.keys(schema)
.filter(key => !schema[key](object[key]))
.map(key => new Error(`${key} is invalid.`));
};
```
why someone just write Overly Complex 2 timesmy first thought was that the assessments quoted seem to be from more than one someone. At least until you see reason to assume someone tries to hurt you (how unprofessional!), think non-appreciative. \$\endgroup\$