Skip to content
This repository was archived by the owner on Jul 28, 2019. It is now read-only.

Web Dev Simplified Changes #78

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

WebDevSimplified
Copy link

Here are most of the changes I would make to clean up your code.
A few notes.

  1. Try not to wire up your event listeners in the HTML. It makes things harder to follow and splits your JS between the HTML and JS files.
  2. Try to use intermediate variables with meaningful names instead of comments to explain complex processes.
  3. Use the Stripe checkout process I used in my Node.js Stripe Checkout video. It is for building custom Stripe buttons which is what you are doing.
  4. I extracted out the getting of all the elements into the initial load of the document instead of each time a method is called. This can be further sped up by lazy loading these elements when they are first accessed, but that is probably overkill.
  5. Try to make smaller methods that do exactly one thing. Many of your methods would do more than one thing. For example your UpdateTotal method would do all of your calculations and text updates. I split the calculations into their own methods and the text updates into their own methods so that one method does not mess with the view and the logic.
  6. Use JS naming conventions. I can tell you have a C# background since this code is formatted very C# like. I would recommend using something like Prettier to auto-format your code so you can get used to the JS way of formatting. It isn't too different from C#.

@MarcusOtter
Copy link
Contributor

MarcusOtter commented Feb 24, 2019

Thank you very much for this pull request! ❤️
It will be merged after the page has officially been released.

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

Successfully merging this pull request may close these issues.

2 participants