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

London 9 - Lovelace - Zobeir - JS-Core-1 - Week 1 #418

Open
Zobeir-Rigi wants to merge 9 commits intoCodeYourFuture:masterfrom
Zobeir-Rigi:master
Open

London 9 - Lovelace - Zobeir - JS-Core-1 - Week 1 #418
Zobeir-Rigi wants to merge 9 commits intoCodeYourFuture:masterfrom
Zobeir-Rigi:master

Conversation

@Zobeir-Rigi
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:Zobeir
  • Your City:london
  • Your Slack Name:Zobeir

Homework Details

  • Module:
  • Week:

Notes

  • What did you find easy?

  • What did you find hard?

  • What do you still not understand?

  • Any other notes?

Copy link
Copy Markdown

@lukedsouza99 lukedsouza99 left a comment

Choose a reason for hiding this comment

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

Really good work Zobeir! 99% of my comments are about formatting which is something that you just learn over time, but the actual content was very good!

Comment thread exercises/B-hello-world/exercise.js Outdated
console.log("Hello world");
console.log("Hello world"+" I just started learning JavaScript!");
console.log(3);
console.log(`Its a test`); No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey Zobeir, this is really good! One very very minor suggestion is to use " " for strings (like you did in line 1) instead of ' ' . Whilst it works in most cases, there are some scenarios where it doesn't like ' '

console.log(greeting);
let greeting=`Hello World`;
for(let i=0; i<=2; i++){
console.log(greeting,`/n`);
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 use of for loop! some more very very minor suggestions, when doing loops/ifs/elses, try indenting the code in the loop block with the tab key. This doesn't affect the code, just makes it easier to read!

@@ -1,3 +1,4 @@
// Start by creating a variable `message`
let message="this is a string";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Another tiny suggestion for readability, when using = add spaces to each side ie let message = "this is a string";

@@ -1,3 +1,4 @@
// Start by creating a variable `message`

let myname="zobeir";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

When naming variables, the general practice is to use something called camel case, where the first letter of a new word is capitalised ie myname would be myName. Again, this is just for readability and doesn't actually change the code.

@@ -1,2 +1,10 @@
var numberOfStudents = 15;
var numberOfMentors = 8;
total=numberOfStudents+numberOfMentors;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

forgot to mention on the = sign space rule, this is the same with other symbols ie total=numberOfStudents+numberOfMentors could be total = numberOfStudents + numberOfMentors. (Again, purely for readability!)

}

function multiply(a, b, c) {
a * b * c;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Very good! Is this line required any more?

@@ -1,16 +1,19 @@
// Add comments to explain what this function does. You're meant to use Google!
/* The math.random() function return a random number between 0 and 1 but when we multiply by 100 it return a random number between 0 and 100;*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great! Commenting is probably the most boring part of the job but it's necessary!

function concatenate(firstWord, secondWord, thirdWord) {
// Write the body of this function to concatenate three words together.
// Look at the test case below to understand what this function is expected to return.
return `${firstWord} ${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.

This works! Another way to do it could be:
var firstAndSecondWord = firstWord.concat(secondWord);
var firstAndSecondAndThirdWord = firstAndSecondWord.concat(thirdWord);

or you could chain them together:
return firstWord.concat(secondWord).concat(thirdWord);

or another way would be to use the function above:
var firstAndSecondWord = combine2Words(firstWord, secondWord);
var firstAndSecondAndThirdWord = combine2Words(firstAndSecondWord, thirdWord);

or chain this:
return combine2Words(combine2Words(firstWord, secondWord), thirdWord);

There are usually loads of ways to do the same thing. As long as it works and is readable then any will do!

Comment thread mandatory/4-tax.js
function calculateSalesTax(price) {
let tax= price+(price*20)/100;
return tax;
}
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!

Comment thread mandatory/4-tax.js

function addTaxAndFormatCurrency() {}
function addTaxAndFormatCurrency(price) {
return `£ ${(price + (price * 20) / 100).toFixed(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.

Well done, this works however I think you're overcomplicating things! You've already written the function that calculates the sales tax value so you can use that instead of rewriting it out!

Something like:

return "£" + calculateSalesTax(price);

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