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

Conversation

AdrianIlovan
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?

@Dedekind561
Copy link
Contributor

Dedekind561 commented Mar 1, 2023

Morning @AdrianIlovan 🌞 ☕ ,
Thankyou for your submission of the first piece of JS1 Week 1 coursework!
Before continuing, please can you update the title of your PR ( Pull Request ). You can follow the instructions in this guide under the section: I want to send my code to volunteers (Pushing) - they give instructions on how to format your PR title.

@@ -1,16 +1,16 @@
// There are syntax errors in this code - can you fix it to pass the tests?

function addNumbers(a b c) {
function addNumbers(a ,b ,c) {
Copy link

Choose a reason for hiding this comment

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

are you using prettier formatter to format your code on save? I recommend it, as it will fix issues like this where the space should be after the comma not before

function combine2Words(word1, word2) {
return word1.concat(word2);
}

function concatenate(firstWord, secondWord, thirdWord) {
return firstWord.concat(" ", secondWord.concat(" ", thirdWord));
Copy link

@ewintram ewintram Mar 4, 2023

Choose a reason for hiding this comment

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

you can also add all arguments into the .concat() method call, e.g.

return firstWord.concat(" ", secondWord, " ", thirdWord);


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

Choose a reason for hiding this comment

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

Great use of interpolation

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

function calculateSalesTax() {}
function calculateSalesTax(number) {
let priceOfProduct = number * 1.2;
Copy link

Choose a reason for hiding this comment

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

you could use a variable name for the number 1.2 so it's more meaningful, e.g.

const totalPlusTax = 1.2;
let priceOfProduct = number * totalPlusTax;
return priceOfProduct;

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

function addTaxAndFormatCurrency() {}
function addTaxAndFormatCurrency(pound) {
pound = calculateSalesTax(pound);
Copy link

Choose a reason for hiding this comment

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

It's better practice to declare a new variable, rather than reassign the value of a parameter, e.g.

const priceWithTax = calculateSalesTax(pound);

function addTaxAndFormatCurrency() {}
function addTaxAndFormatCurrency(pound) {
pound = calculateSalesTax(pound);
return `£${pound.toFixed(2)}`;
Copy link

Choose a reason for hiding this comment

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

👍 very nice use of .toFixed() and string interpolation

@@ -1,14 +1,15 @@
// Add comments to explain what this function does. You're meant to use Google!
// It generates a random number using Math.random() and it multiplies by 10
Copy link

Choose a reason for hiding this comment

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

Math.random() returns a number greater than or equal to 0 and less than 1, so this returns a number greater than or equal to 0 and less than 10

@AdrianIlovan AdrianIlovan changed the title Lnd10/adrian ilovan LND10 | Adrian Ilovan | JS1 Coursework Week1 Jun 6, 2023
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.

3 participants