Skip to content
This repository was archived by the owner on Jan 14, 2024. It is now read-only.

Glasgow Class 6 - Cat Smith -JS1-Week 1 #538

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

Conversation

CatSmith1
Copy link

Volunteers: Are you marking this coursework? You can find a guide on how to mark this coursework in HOW_TO_MARK.md in the root of this repository

Your Details

  • Your Name:
  • Your City:
  • Your Slack Name:

Homework Details

  • Module:
  • Week:

Notes

  • What did you find easy?

  • What did you find hard?

  • What do you still not understand?

  • Any other notes?

@CatSmith1
Copy link
Author

Cat Smith
Glasgow
CatSmith1

Module: JS1
Week 1

Copy link
Contributor

@Dedekind561 Dedekind561 left a comment

Choose a reason for hiding this comment

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

Hello @CatSmith1 ,
Well done on submitting your first piece of JS coursework.
My main piece of advice is around variable names and declarations - I've left a few comments for you to think about.
Good job!


return "The total is total";
return `The total is ${total}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice job using template literals here 👍

@@ -5,7 +5,13 @@
Sales tax is 20% of the price of the product.
*/

function calculateSalesTax() {}
function calculateSalesTax(a) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You've used a as a parameter name here. But this doesn't offer insight into what the parameter represents. Remember your code is designed to be read by others so always try to convey meaning when you choose variable names - sometimes this can be tricky!

@@ -5,7 +5,13 @@
Sales tax is 20% of the price of the product.
*/

function calculateSalesTax() {}
function calculateSalesTax(a) {
subTotal = a * .2;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
subTotal = a * .2;
let subTotal = a * .2;

When declaring your variables, you've ommitted the let/const keyword. This will still work but for reasons you'll learn about later this is bad practice ( you'll almost always declare variables with let/const )

@@ -17,7 +23,12 @@ function calculateSalesTax() {}
Remember that the prices must include the sales tax (hint: you already wrote a function for this!)
*/

function addTaxAndFormatCurrency() {}
function addTaxAndFormatCurrency(a) {
subTotal = a * .2;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing here with let/const

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