forked from AdaGold/FarMar
-
Notifications
You must be signed in to change notification settings - Fork 26
FarMar #48
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
Open
sophiabaldonado
wants to merge
29
commits into
Ada-C5:master
Choose a base branch
from
sophiabaldonado:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
FarMar #48
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…cts based on csv data; one test for each;
…s a given id for all classes;
…with that instance of Market;
…f vendor sells at;
… an id rather than array index;
… instance; Created sales method finds all sales associated with a vendor instance; Created market spec to return markets that contain nil values for any of their instance varaiables;
…ess you want them to; Created methods for Market to find markets with missing data;
…th an instance of Product;
…is instance of product;
… of product has been referenced in a sale;
…ces associated with a vendor id;
…d vendor and product for an instance of Sale;
…purchase time within two given times, uses DateTime;
…using the find method to test on instances of each class; This saves time because it doesn't have to call the find enumerable for each test and keeps the test data the same even if the csv changes;
…ty and wrote better specs for it; The method now creates an array that includes the market id and what info is missing, spec doesn't print it out anymore;
…ed the find methods when I added hard-coded instances lol; Refactor specs to use let where appropriate;
…d of twice; Uses an each loop instead of map and zip enumerables;
…or; Added new specs for .all for each class;
…the instance of market based on its vendors;
…ndor names and return a collection of markets that include your search term;
… 7 minutes to execute.. ; Cleaned up market_spec a little;
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Worked on by Sophia Baldonado.
Requires awesome_print for enhanced specs.
The between method for the Sale class was interesting to think about and I ended up doing it in a way that's probably more complicated than needed but makes sense to me. The most frustrating optional was the most_revenue method in the Vendor class because it takes a really long time to run -- I skip the test for it because it takes several minutes to execute -- but I did learn a better way to implement it, which would require a large refactor of all of the classes. I might do that over the break!