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

London Class 7 - Benyam Worku - Javascript-Core - Week 1#71

Closed
BenyamWorku wants to merge 5 commits into
CodeYourFuture:masterfrom
BenyamWorku:master
Closed

London Class 7 - Benyam Worku - Javascript-Core - Week 1#71
BenyamWorku wants to merge 5 commits into
CodeYourFuture:masterfrom
BenyamWorku:master

Conversation

@BenyamWorku
Copy link
Copy Markdown

Your Details

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

Homework Details

  • Module:
  • Week:

@AhmadJanah
Copy link
Copy Markdown

Hi Benyam, You have done a great job as far as I check, everything looks good. I suggest that may some comments will give make your code more readable and make it easier to understand.
All the best!

@BenyamWorku
Copy link
Copy Markdown
Author

Hi Benyam, You have done a great job as far as I check, everything looks good. I suggest that may some comments will give make your code more readable and make it easier to understand.
All the best!

Thanks Ahmad I have now added some more comments.

Copy link
Copy Markdown

@AbdulazizAlhasan AbdulazizAlhasan left a comment

Choose a reason for hiding this comment

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

Well done Benyam

Comment thread mandatory/4-tax.js
Comment on lines +23 to +25
let afterTax = prices + calculateSalesTax(prices);
return `£${afterTax}`
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you might need to use .toFixed(2) to force it to give two decimals

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.

Yeah that is a good idea. But , this passed the test too. I guess it is because we've multiplied it by 0.2 . I will look into it. Thanks !

Copy link
Copy Markdown

@amellie amellie left a comment

Choose a reason for hiding this comment

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

Very well done on your effort, Ben! If you could please correct some of the comments I have for you? :)

}

function concatenate(firstWord, secondWord, thirdWord) {
return firstWord + " ".concat(secondWord) + " ".concat(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 perfectly well, but I would advise you to use one method of concatenating. In this line, you use both the plus sign + and the concat method. Since both methods are the same, it is always advised to use the plus sign as it is easier to read. Can you please change this line of code to reflect this?

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.

Sure will do thanks .

Comment thread mandatory/4-tax.js

function calculateSalesTax() {}
function calculateSalesTax(prices) {
return 0.2 * prices;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It seems that the test has failed. Would you be able to fix this?

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.

Has it ? It passed on my machine . I will apply toFixed(2)

Comment thread extra/3-magic-8-ball.js
*/
function checkAnswer(answer) {
//Write your code in here
for (const group of Object.keys(magicAnswer))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just a small note here. Please ensure that the indentation is correctly done (especially the brackets). It should be:

for(...) {
    //code 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.

Prettier has been working fine . I will have a look. Thanks !

Copy link
Copy Markdown

Choose a reason for hiding this comment

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


// Add comments to explain what this function does. You're meant to use Google!
function s(w1, w2) {
// stringing together two strings-no spaces- and returns 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.

A better way of commenting is: join two strings with no spaces

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.

Okay thanks

@github-actions
Copy link
Copy Markdown

Your coursework submission has been closed because nobody has interacted with it in six weeks. You are welcome to re-open it to get more feedback.

@github-actions github-actions Bot added the Stale label May 13, 2021
@github-actions github-actions Bot closed this May 13, 2021
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.

4 participants