Conversation
jlcarmic
left a comment
jlcarmic
left a comment
There was a problem hiding this comment.
Could be cleaner. Dealing with orders one at a time instead of passing around the entire book would have made this much easier to solve and made your code more readable and maintainable. The only instance you'd even need to look at a second order is if the incoming order was partially fulfilled so why loop past it when you don't have to which is what your filters are doing. In fact you are looping through the entire array twice before even determining what your data looks like and if you need to work with it at all.
Score: 86 + 10 = 96%
Explainer Video: https://drive.google.com/open?id=10t8tsCqw_ESVrCTcEjTAmL9MevQu0bv9
| function reconcileOrder(existingBook, incomingOrder) { | ||
| const oppOrderTypes = existingBook.filter(order => order.type !== incomingOrder.type) | ||
| let updatedBook = existingBook.filter(order => order.type === incomingOrder.type) | ||
| updatedBook = oppOrderTypes.length |
There was a problem hiding this comment.
If you're gonna short circuit on the existingBook.length you should do that before you start filtering.
| function reconcileOrder(existingBook, incomingOrder) { | ||
| const oppOrderTypes = existingBook.filter(order => order.type !== incomingOrder.type) | ||
| let updatedBook = existingBook.filter(order => order.type === incomingOrder.type) | ||
| updatedBook = oppOrderTypes.length | ||
| ? updatedBook.concat(determineSamePrice(oppOrderTypes, incomingOrder)) | ||
| : updatedBook.concat(incomingOrder) | ||
| return updatedBook | ||
| } |
There was a problem hiding this comment.
If you dealt with one order at a time you wouldn't need so many loops. You're filtering twice here and then later doing a findIndex. See the solution branches.
| else if (existingBook[index].quantity < incomingOrder.quantity) { | ||
| incomingOrder.quantity -= existingBook[index].quantity | ||
| existingBook.splice(index, 1) | ||
| return determineSamePrice(existingBook, incomingOrder) |
There was a problem hiding this comment.
You've got flow of control split out across multiple functions which is why this code becomes so unreadable. It also makes debugging much harder. reconcileOrder should be responsible for flow of control with the other functions merely operating on and returning data for it.
| { type: 'buy', quantity: 15, price: 5900 }, | ||
| { type: 'sell', quantity: 15, price: 6000 } |
There was a problem hiding this comment.
You shouldn't have to change the order of these if written correctly.
2 participants
No description provided.