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

Glasgow Class 6 - Man Sang Sin - JS1 - Week 1 - #497

Open
ManSangSin wants to merge 5 commits into
CodeYourFuture:masterfrom
ManSangSin:master
Open

Glasgow Class 6 - Man Sang Sin - JS1 - Week 1#497
ManSangSin wants to merge 5 commits into
CodeYourFuture:masterfrom
ManSangSin:master

Conversation

@ManSangSin

Copy link
Copy Markdown

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: Man Sang Sin
  • Your City: Glasgow
  • Your Slack Name: Man Sang Sin

Homework Details

  • Module: JavaScript Core 1
  • Week: 1

Notes

  • What did you find easy?

  • console.log(), variables, strings, basic calculations

  • What did you find hard?

  • 8 ball homework

  • What do you still not understand?

  • Any other notes?

- completed tasks 1 - 4
- completed tasks 1 - 3

@beardedslav beardedslav left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good job on completing the coursework! Let me know if you have any further questions on my comments!

Comment thread extra/1-currency-conversion.js Outdated
function convertToBRL(amountGBP) {
convertedAmount = amountGBP * 0.99 * 5.7;
convertedAmount = convertedAmount.toFixed(2);
return Number(convertedAmount);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Well done! This code works as needed, but you are converting a number to a string and then again to a number. Can you think of a way to do achieve the same result by using only math operations?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

math.round() seems to be a better method.

convertedAmount = (Math.round(convertedAmount * 100)/100)

Comment thread extra/2-piping.js Outdated

// Why can this code be seen as bad practice? Comment your answer.
let badCode =
let badCode = `£${((startingValue + 10) * 2)}`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this is a good example of a bad code, but the assignment was to "...perform the following operations using your functions all on one line". You could start it like so:
let badCode = format(multiply(add(
How would you complete it?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

let badCode = format(multiply(add(startingValue, 10)));

Comment thread extra/2-piping.js Outdated
Comment thread extra/3-magic-8-ball.js Outdated
function shakeBall() {
//Write your code in here
let answers = veryPositive.concat(positive, negative, veryNegative);
let randomInteger = Math.floor(Math.random() * 25);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

That's a nice way to put all the answers in one array. You are concatenating 4 arrays of 5 elements each, are you sure 25 is the best choice for this line? Also what would happen if you were asked to add 2 more answers to every answer type? Is there a way to not to have to rely on a hardcoded number like 25 here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

20 would be a better value (would give a range between 0 - 19). I think I must've miscounted 5 arrays.
As for not hardcoding the value, counting the variables in the array would be a better option?
I tested out:

let numberOfAnswers = answers.length;
let randomInteger = Math.floor(Math.random() * numberOfAnswers);

and it seems to work.

return Math.random() * 10;
}

// Picks a random number between 0 (inclusive) and 1 (non-inclusive) and multiplies it by 10.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is a good explanation of what happens inside the getRandomNumber() function. How would you describe the end result of calling this function?
Also it's a good practice to add a comment explaining function's behaviour either above or inside the function, rather than below it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Returns a value which is between 0 and (but not including) 10.

// Joins the variable string of word1 to word2 exactly as shown.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice, this is a correct implementation. Is there a way to achieve the same end result without using concat method?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

return ${firstWord} ${secondWord} ${thirdWord}

Comment thread mandatory/4-tax.js Outdated
Comment thread mandatory/4-tax.js Outdated

function calculateSalesTax() {}
function calculateSalesTax(amount) {
return amount = ((amount * 0.2) + (amount));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good implementation, but can it be expressed more concisely with fewer maths operations?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

return amount = (1.2 * amount)

@ManSangSin

Copy link
Copy Markdown
Author

Good job on completing the coursework! Let me know if you have any further questions on my comments!

Thanks for checking over my work and providing some very useful hints/tips! Your feedback was very helpful.

ManSangSin and others added 3 commits February 19, 2023 21:49
change line to call function

Co-authored-by: Krzysztof Malinowski <krzysztof+github@malinowski.scot>
mandatory/4 - shortened math function
extra/1 - used Math.round() instead of .toFixed() so no need to convert string to number
extra/2 - changed code to call function
extra/3 - used array.length() to count array instead of hardcoding number of array items
Co-authored-by: Krzysztof Malinowski <krzysztof+github@malinowski.scot>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants