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

Sara Chandler -- Carets #33

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

Conversation

SaraChandler
Copy link

Ada Trader

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
How do Backbone Views help structure your code? It helped structure the code into an MVC pattern, making organization easier.
Did you use jQuery directly in your Views? How, and why? I used it as a selector to empty sections of the HTML.
What was an example of an event you triggered? Why did you need to trigger it? I triggered an event when 'buy' and 'sell' were clicked so the trade history could be updated with that specific quote information.
In what was is unit testing in JavaScript similar to unit testing in Ruby? In what ways is it different? You can test for similar things, like validations and model logic. The syntax is a bit different.

@CheezItMan
Copy link

Ada Trader

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commits, and good commit messages
Comprehension questions Check, but what do View specifically add, Also you didn't use jQuery directly in your views, instead you used this.$ to select within the view and avoid bugs where your view is interacting with elements outside it's purview.
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 It detects changes in the Quotes and removes itself when the trade executes, but it does NOT update the trading history. I left a note in your code.
General
Has separate views for different parts of the app buyOrder
Uses events (listening/handling and triggering) to manage different behavior in views Yes, although see my above bug. I left a note in your code how you could handle it using the event Bus.
Practices good standards for views (ES6 syntax, doesn't directly use jQuery, returns this in render) Well done here, I will say that you could write buyTrade(event) { instead of buyTrade: function(event) {. You're doing things the old-fashioned way.
Error handling for the Order Entry Form Well done, good use of validate.
Testing
Has unit tests for models MISSING
Overall Nicely done, you hit all the learning goals of the project except for testing. See my in-code notes, but this was good work!

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