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

LONDON9-LOVELACE-BOSHRA-MAHMOUDI-JS1-WEEK1 - #426

Open
BoshraM wants to merge 2 commits into
CodeYourFuture:masterfrom
BoshraM:master
Open

LONDON9-LOVELACE-BOSHRA-MAHMOUDI-JS1-WEEK1#426
BoshraM wants to merge 2 commits into
CodeYourFuture:masterfrom
BoshraM:master

Conversation

@BoshraM

@BoshraM BoshraM commented Nov 22, 2022

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:
  • 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
Dedekind561 self-requested a review December 1, 2022 19:21
@@ -1,3 +1,5 @@
// Start by creating a variable `greeting`

var greeting = "I am BOshra";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi there @BoshraM,
We've just updated our coursework with a change to the variable declarations.
From now on when you're writing variable declarations be sure to use let / const over var.

Suggested change
var greeting = "I am BOshra";
const greeting = "I am BOshra";


let greeting = "My name is";
let myName = "Boshra";
let message = greeting + " " + myName;

@Dedekind561 Dedekind561 Dec 1, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good.
How else do you think you can improve the RHS ( Right Hand Side ) on line 4 though? You're currently using + to concatenate the strings together - but what other techniques can you use for forming strings using variables. Try checking out template literals and see if you can refactor your code :)

var numberOfMentors = 8;
let totalNumber = numberOfMentors + numberOfStudents;
let percentageOfStudents = (numberOfStudents / totalNumber) * 100;
let roughPercentageOfStudents = Math.round(percentageOfStudents);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why did you decide to call this variable roughPercentageOfStudents in this case - what makes it rough?


console.log(greeting);

var greeting = createGreeting("Boshra");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👓 Watch out for var here, again!

}

function greeting(mentor) {
return `HELLO ${upperCaseName(mentor)}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice use of template literals here! 😄

Comment thread mandatory/4-tax.js
function addTaxAndFormatCurrency() {}

function addTaxAndFormatCurrency(price) {
return `£${calculateSalesTax(price).toFixed(2)}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Possibly think about adding another variable in here to make the final return value a bit more readable.
Which part of the expression calculateSalesTax(price).toFixed(2) could you lift out of the template literal and give a meaningful name to ?

function convertToBRL() {}

function convertToBRL(price) {
let newPrice = price * 0.99 * 5.7;

@Dedekind561 Dedekind561 Dec 1, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This solution looks good to me - I like your use of multiplication by 0.99 to reduce the original value by 1%. 😎
Think about this function for a moment from the perspective of somebody who perhaps is a bit less familiar.
They probably won't have much idea of what the 5.7 means.
Could you assign 5.7 to a variable with a clear name showing what it represents ?

@Dedekind561 Dedekind561 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @BoshraM,

Good job on completing your first piece of JS coursework at CYF! 🎉

I'm seeing lots of good solutions in your coursework so far, so keep up the effort. I think my main piece of feedback would be to think about using some extra variables to store parts of the more complex expressions in your functions. Using variables like this will improve the readability of your code from day 1.

Nice work! 👍

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