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

London 10-Mahendra Balal-javascript core 1-coursework-week2 #445

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mabalal
Copy link

@mabalal mabalal commented Mar 1, 2023

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?

Comment on lines 14 to 16
if (isHappy) {
return "I am happy";
} else {
Copy link

Choose a reason for hiding this comment

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

There's a pattern in programming to simplify if .. else statements. You can rewrite this block as follows:

 if (isHappy) {
    return "I am happy";
  }

return "I am not happy";

the pattern is called return early pattern, and you can read more here https://medium.com/swlh/return-early-pattern-3d18a41bba8

Comment on lines +44 to +52
function printOddNumbers(limit) {
let i = 1;
while (i <= limit) {
if(i % 2 !==0) {
console.log(i);
}
i++;
}
}
Copy link

Choose a reason for hiding this comment

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

this works fine, but you could make it 2x more performant by skipping over even numbers. For example:

Suggested change
function printOddNumbers(limit) {
let i = 1;
while (i <= limit) {
if(i % 2 !==0) {
console.log(i);
}
i++;
}
}
function printOddNumbers(limit) {
let i = 1;
while (i <= limit) {
console.log(i);
i+=2;
}
}

Comment on lines +59 to +61
if (price1 < 0 || price2 < 0) {
return "Invalid price for an item";
}
Copy link

Choose a reason for hiding this comment

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

Nice add, always good to handle edge cases even if the exercise didn't mention it

Comment on lines +63 to +67
let totalPrice = price1 + price2;
let discount = Math.min(price1, price2);
let discountedPrice = totalPrice - discount;

return discountedPrice;
Copy link

Choose a reason for hiding this comment

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

I think this can be simplified with the following:

Suggested change
let totalPrice = price1 + price2;
let discount = Math.min(price1, price2);
let discountedPrice = totalPrice - discount;
return discountedPrice;
return Math.max(price1, price2);

What do you think?

Copy link

@berkeli berkeli left a comment

Choose a reason for hiding this comment

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

Nice work @mabalal, I left a few minor comments

@berkeli berkeli added the reviewed A mentor has reviewed this code label Mar 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
reviewed A mentor has reviewed this code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants