-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Jan Edrozo | Carets #27
base: master
Are you sure you want to change the base?
Conversation
…r functions; Updated app.js(added a quoteListView variable to render)
…js Update: Added buyQuote & sellQuote button click events
…s so trade history entry records correct price (price before trade entry is logged)
… in order model, order_list collection, and their respective views
…l methods in quote model and changed showTrade function to pass tradeData instead of a quoteView in quote_list_view.js
…and sell() methods
… in buy() and sell() methods
…appended to index div.form-errors
…Order method; NOTE: priceCheck() is the callback function for an event listener that each order model has to monitor the changes in the Quote price to match the target price conditions
…o invoke the matchedQuote methods: buy() or sell()
…der callback function
Ada TraderWhat We're Looking For
|
// console.log(newOrder.matchedQuote); | ||
if (newOrder.isValid()) { | ||
this.model.add(newOrder); | ||
newOrder.listenTo(newOrder.get('matchedQuote'), 'change', newOrder.priceCheck); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! 🥇
'click button.btn-buy': 'buyOrder', | ||
'click button.btn-sell': 'sellOrder', | ||
}, | ||
buyOrder: function(event) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't help but think you could refactor these methods to reuse the code.
|
||
it("buys if quote's price is equal to or below order's targetPrice", () => { | ||
listener.listenTo(buyOrder, 'destroy', () => { | ||
expect(true).toBeTruthy(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part doesn't actually test anything since it only runs the test if destroy was called. You'd instead probably want to use a spy (part of Jasmine) and check to see if the event handler had been called.
Good work trying to test these however.
}); | ||
sellOrder.get('matchedQuote').set('price', 101); | ||
sellOrder.priceCheck(); | ||
expect(testOrderList.length).toEqual(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good place to do the testing for destroying models however, nice work!
model: quotes, | ||
template: _.template($('#quote-template').html()), | ||
tradeTemplate: _.template($('#trade-template').html()), | ||
el: 'main', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit of a problem, it leads your QuoteListView
instance and OrderListView
instance to overlap on the DOM, which means they could interfere with each other.
Best practices in Backbone is for independent views to NOT overlap in the DOM. You can have subviews, but views that don't know about each other (one isn't managing the other) should not overlap.
Ada Trader
Congratulations! You're submitting your assignment!
Comprehension Questions