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

Raymond's Car makes all code beautiful. #2

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

Conversation

danielgultom
Copy link

Description of changes:

Give a description of what behavior you've created or bug that you've fixed

  • more descriptive commenting
  • better naming:
    • use _id instead of items_id
    • rename amount_spent to customer_to_amount_spent
    • Change to underscore naming convention
    • rename REWARDS_RATIO_BELOW to REWARDS_RATIO_BELOW_CUTOFF
    • add class variable for REWARDS_RATIO_ABOVE_CUTOFF (17)
  • file organization:
    • import statements at top of file
    • move small function above
    • restructure if-else structure for rewards calculation to set ratio rather than explicitly calculate
  • code concision:
    • remove unnecessary total amount calculation; update dictionary directly
    • use unary operator += instead of dict.get()
    • detect if customer_id is present rather than the negative case, eliminating a clause
    • add item price directly to customer total, not declaring new variable
    • use dictionary.items() to iterate through for loop
  • straight-up mistakes:
    • remove erroneous multiplication by item.id
    • remove unnecessary print statement
    • check if items_purchased is empty first since it should throw an error

Which files were touched:

  • rewardPointsSystem.py

Notes for PMs:

We made the code beautiful.

Screenshots (if frontend change):

Only necessary for CSS/styling changes

Relevant Issue:

jodielu#3

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