Skip to content
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

Lauren Cardella -- Carets #35

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Conversation

enigmagnetic
Copy link

Ada Trader

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
How do Backbone Views help structure your code? They act as a controller, routing messages to and from the models, the DOM, and other views.
Did you use jQuery directly in your Views? How, and why? I did not use any "direct" jQuery in my views, only this.$ notation. I did this to ensure the elements selected belong to proper view scope.
What was an example of an event you triggered? Why did you need to trigger it? In the quote view, an event 'checkSymbolName' is triggered, which is interpolated with the symbol of the view model. This is needed to alert the order view that it should check its model's targetPrice against the corresponding quote's price. The order view is listening only for the event that corresponds to its own symbol name, ensuring only the proper quote can be traded once the conditions are met.
In what ways is unit testing in JavaScript similar to unit testing in Ruby? In what ways is it different? The structure of the tests are very similar, with the nested describe blocks, and it blocks. The setup-execute-assert flow is also similar, including the dot notation used in the expect statements. The actual syntax is different. It's also harder to test controller actions because so much of it includes responding to user interaction or manipulating the DOM, which is browser-dependent.

…nd buyQuote and sellQuote methods to quote view.
…y and sell methods in Quote model. Build addTrade function in QuoteListView. Add tradeTemplate to QuoteListView.
…ow fixed. Adjusted quote to trigger tradeMe for the quote list view.
…ist and order model/view/listeners are remvoed as expected. Bug: the trades are made near but not actually above or below the target price.
@CheezItMan
Copy link

Ada Trader

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good commit messages and good number of commits.
Comprehension questions Check
Organization
Models and collections are defined in separate files Check
Code that relies on the DOM is located in or called by $(document).ready Check
Functionality
Quote prices change when clicking Buy and Sell Check
The Trade History updates when buying and selling a quote Check
A user can create an open order using the Order Entry Form Check
An open order removes itself from the open orders and updates the Trade History when fulfilled Check, however you're having a lot of the business logic in the views rather than the Order model. Also you have an event bus but aren't using it as far as I can tell.
General
Has separate views for different parts of the app Check
Uses events (listening/handling and triggering) to manage different behavior in views Check, although see my note above
Practices good standards for views (ES6 syntax, doesn't directly use jQuery, returns this in render) Check
Error handling for the Order Entry Form Check
Testing
Has unit tests for models Has basic tests
Overall Not bad, you hit the major learning goals, just look at where you have business logic and maybe the bus can be removed.

events: {
'click button.btn-cancel': 'cancelOrder',
},
checkQuote: function(quote) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like business logic and would make some sense to put into the Model

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants