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

London_10-Anna_Hrychaniuk-JS-Core-1-Week1 - #495

Open
AnnaFYZ wants to merge 4 commits into
CodeYourFuture:masterfrom
AnnaFYZ:master
Open

London_10-Anna_Hrychaniuk-JS-Core-1-Week1#495
AnnaFYZ wants to merge 4 commits into
CodeYourFuture:masterfrom
AnnaFYZ:master

Conversation

@AnnaFYZ

@AnnaFYZ AnnaFYZ commented Feb 19, 2023

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: Anna Hrychaniuk
  • Your City: London10
  • Your Slack Name: Anna Hrychaniuk

Homework Details

  • Module: JS-Core-1
  • Week: 1

Notes

  • What did you find easy?

  • What did you find hard?

  • What do you still not understand?

  • Any other notes?

Comment thread extra/3-magic-8-ball.js Outdated
Comment on lines +62 to +65
const allAnswers = ["very positive", "positive", "negative", "very negative"];
let answer = allAnswers [Math.floor(Math.random()*allAnswers.length)];
// console.log(prediction);
return answer;

@maxbmaapc maxbmaapc Feb 19, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
const allAnswers = ["very positive", "positive", "negative", "very negative"];
let answer = allAnswers [Math.floor(Math.random()*allAnswers.length)];
// console.log(prediction);
return answer;
const allAnswers = [...veryPositive, ...positive, ...negative, ...veryNegative];
let answer = allAnswers[Math.floor(Math.random()*allAnswers.length)];
return answer;

Have a look at spread operator and try to remove unnecessary comments.

Comment thread extra/3-magic-8-ball.js
Comment on lines +79 to +88
if (veryPositive.indexOf(answer) >= 0 ) {
console.log("very positive");
//Write your code in here
} else if (positive.indexOf(answer) >= 0) {
console.log("positive");
} else if (negative.indexOf(answer) >= 0) {
console.log("negative");
} else {
console.log("very negative");
}

@maxbmaapc maxbmaapc Feb 19, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
if (veryPositive.indexOf(answer) >= 0 ) {
console.log("very positive");
//Write your code in here
} else if (positive.indexOf(answer) >= 0) {
console.log("positive");
} else if (negative.indexOf(answer) >= 0) {
console.log("negative");
} else {
console.log("very negative");
}
if (veryPositive.indexOf(answer) >= 0 ) {
return "very positive";
} else if (positive.indexOf(answer) >= 0) {
return "positive";
} else if (negative.indexOf(answer) >= 0) {
return "negative";
} else {
return "very negative";
}

console.log is necessary only for developer to see what's happening. As I see you just forgot to have "return" in your loop.

Comment thread extra/3-magic-8-ball.js
}


console.log(checkAnswer(shakeBall()));

@maxbmaapc maxbmaapc Feb 19, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
console.log(checkAnswer(shakeBall()));

@maxbmaapc

Copy link
Copy Markdown

Good job anyway, weel done with the length.


function convertToBRL() {}
function convertToBRL(amount) {
return parseFloat((amount * 0.99 * 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.

This is a clever solution and I like your use of parseFloat, well done! However, it is not that readable. Could you break it down into steps - perhaps working out the price after fee first.

Also, could you be more specific with the labelling? i.e. instead of amount, GBP

*/

function convertToUSD() {}
function convertToUSD(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.

Nice! But could you be clearer with the name 'amount'?

// There are syntax errors in this code - can you fix it to pass the tests?

function addNumbers(a b c) {
function addNumbers(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.

Nice!

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

Great work, Anna! Well done on tackling the extra challenges. I can see you're good at keeping your code tidy and have mastered string interpolation.

Try and work on breaking out your code to more digestable steps, ensuring that one line tackles one small part of the problem at a time. That way, when your future colleagues are checking your code they'll be able to understand it quickly :)

total = a ++ b;

return "The total is total";
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.

Nice use of string interpolation

function multiply(a, b, c) {
a * b * c;
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

watch this trailing space, keep it tidy :)

Comment thread extra/2-piping.js

function format() {
function format(digit) {
return `£${digit.toString()}`;

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 thinking, but do you need the toString?

Comment thread extra/2-piping.js
function multiply() {

function multiply(a, b) {
return 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.

Nice and tidy with spacing and semi-colon, well done :)

Comment thread extra/2-piping.js
Comment on lines +35 to +36
let badCode = format((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.

Could you use all your methods in this bad code example?

Comment thread extra/2-piping.js
let badCode =
let badCode = format((startingValue + 10) * 2);

// functions are not really used, so it would work just for one case

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The badcode is really hard to read and is doing too much on one line so is prone to bugs :)

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

let goodCode =
let goodCode = format(multiply(add(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.

Nice getting this working on one line, but good code is readable in clear steps and stages. Can you break out these functions to call them on different lines?

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

Great work, Anna! Well done on tackling the extra challenges. I can see you're good at keeping your code tidy and have mastered string interpolation.

Try and work on breaking out your code to more digestable steps, ensuring that one line tackles one small part of the problem at a time. That way, when your future colleagues are checking your code they'll be able to understand it quickly :)

Comment thread mandatory/4-tax.js

function calculateSalesTax() {}
function calculateSalesTax(netPrice) {
return netPrice * 1.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.

I am not sure if netprice is exactly correct to pass in (as I think netprice is after tax has been removed, and this is still before) but I like how specific and readable this label is.

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.

3 participants