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

NW4 -Gulnihal Naldoken - JS -Week1 - #125

Closed
gulnihaln wants to merge 20 commits into
CodeYourFuture:masterfrom
gulnihaln:master
Closed

NW4 -Gulnihal Naldoken - JS -Week1#125
gulnihaln wants to merge 20 commits into
CodeYourFuture:masterfrom
gulnihaln:master

Conversation

@gulnihaln

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?

@zeinaji zeinaji 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.

Nice work Gulnihal 😊 and well done on the extensions! The logic is sound across the coursework, my only comment is to try and assign expressions to variables whenever possible just as a way of making the code easier to read

console.log(message);
var greeting = message + firstName;


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 leaving gaps between lines of code, it's best to go for no more than one line at a time to make things easier to read 🙂

console.log(firstNameLength);

var message = "My name is Gulnihal and my name is 8 characters long"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Here you could concatenate the message variable using the other two variables, either with a plus operator + or through interpolation. So could have
var message = "My name is " + firstName + " and my name is " + firstNameLength + " characters long"
or
var message = `My name is ${firstName} and my name is ${firstNameLength} characters long`

var numberOfStudents =30;
var numberOfMentors = 16;
var totalNumberOfStudentsAndMentors = numberOfStudents + numberOfMentors;
console.log(totalNumberOfStudentsAndMentors); 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.

You could also use string concatenation here to log it in this format

Number of students: 15
Number of mentors: 8
Total number of students and mentors: 23

console.log(roughProportionStudents);

var proportionMentors = (numberOfMentors / (numberOfStudents + numberOfMentors)) *100;
console.log(proportionMentors);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Instead of using numberOfStudents + numberOfMentors twice here, you could assign it to a variable like before, so totalNumberOfStudentsAndMentors and reference that instead, it'll make the statement more concise 🙂

function getTotal(a, b) {
total = a ++ b;
total = a + b;

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 also needs a const 🙂


function concatenate(firstWord, secondWord, thirdWord) {
return firstWord.concat(secondWord, thirdWord);
// Write the body of this function to concatenate three words together.

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 can add spaces as separate strings here, for example firstWord.concat(" ", secondWord, " ", thirdWord);


function convertToBRL() {}
function convertToBRL(price) {
let BrlToP = parseFloat((price *(99/100) * 5.7).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.

Because this variable won't change after being assigned it's better to use const here

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also to make it easier to know what the numbers are for, it is better to assign them to variables. So we could have

  const transactionFee = 0.01;
  const exchangeRate = 5.7;
  const amountAfterFee = amountInPounds * (1 - transactionFee);

This is just to help avoid 'magic numbers' https://levelup.gitconnected.com/magic-numbers-820d2d570cc5

Comment thread extra/2-piping.js
/* BETTER PRACTICE */

let goodCode =
let goodCode = "£" + ((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.

It's actually the other way round here, it's better practice to use the functions, as it gives less room for error. However, you could make it more readable, because as you said it looks quite crowded. You could assign the output of each function to a variable and then reference that, it'd make it easier to read. For example:

const startingValueAddTen = add(startingValue, 10);
const multipliedByTwo = multiply(startingValueAddTen, 2);
const formattedValue = format(multipliedByTwo);

Comment thread extra/3-magic-8-ball.js
"Concentrate and ask again.",

// ## Very negative
"Don't count on it.",

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 could also have 4 different array variables here, veryPositive, positive, negative, and veryNegative and concatenating these in the answers one

Comment thread extra/3-magic-8-ball.js
:index <10 ? "positive"
:index <15 ? "negative"
: "very negative";

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 nice use of the ternary operator here 🙂

@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 Aug 16, 2021
@github-actions github-actions Bot closed this Aug 16, 2021
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