Skip to content
This repository was archived by the owner on Apr 18, 2025. It is now read-only.

Conversation

RbAvci
Copy link

@RbAvci RbAvci commented Mar 25, 2024

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

A basic web application that fetches and displays the latest xkcd comic, including an HTML structure, CSS for styling, and JavaScript for API fetching and rendering.

Questions

Ask any questions you have for your reviewer.

@RbAvci RbAvci changed the title NW6 | Rabia Avci | JS3 Module | [TECH ED] Programmer Humour | Week 3 Backlog NW6 | Rabia Avci | JS3 Module | [TECH ED] Programmer Humour | Week 3 Mar 25, 2024
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Comment on lines +74 to +82
console.log('The names of the people who belong to the Gryffindor house:')

hogwarts.filter(({house}) => house === "Gryffindor")
.forEach(({firstName,lastName}) => console.log(`${firstName} ${lastName}`));

console.log('The names of teachers who have pets:')

hogwarts.filter(({pet,occupation})=> pet !== null && occupation === "Teacher" )
.forEach(({firstName,lastName}) => console.log(`${firstName} ${lastName}`));

Choose a reason for hiding this comment

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

Perfect!

Well done

Comment on lines +10 to +24
function formatPrice(price) {
return price.toFixed(2);
}

console.log("QTY\tITEM\t\t\tTOTAL");

let total = 0;

for (const {quantity, itemName, unitPrice} of order) {
console.log(`${quantity}\t${itemName}\t\t${(unitPrice * quantity).toFixed(2)}`);
total += unitPrice * quantity;
}

console.log("\nTotal:", formatPrice(total));

Choose a reason for hiding this comment

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

Well done finding a solution!

Also, an improvement you could make to this solution would be to use Array.reduce() to create a running aggregate instead of defining totalPrice outside of the loop.

Here is a link that explains using reduce to calculate a sum.

https://linuxhint.com/call-reduce-on-an-array-of-objects-to-sum-their-properties/

Also, you have used toFixed() on line 19, but then used the formatPrice function on line 23, I would encourage you to be consistent and either use formatPrice in both instances of remove this function and use toFixed(2) on line 23.

Comment on lines +12 to +37
h1 {
color: aliceblue;
max-width: 100%;
padding: 10px;
background-color: #c24242;

}

img {
max-width: 100%;
height: auto;
box-shadow: 0 0 10px rgba(0, 0, 0, 0.1);
background-color: #c24242;
padding: 2rem;

}

footer {
color: aliceblue;
padding: 1px;
max-width: 100%;
border: solid 2px #c24242;
margin-top: 10px;
background-color: #c24242;

}

Choose a reason for hiding this comment

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

It is not advisable to set CSS values on generic HTML elements such as img, div, h1, h2 etc.

Doing this will mean you will apply these styles to every element you create.

Instead, use classNames on HTML elements to apply styling to specific elements without affecting anything that doesn't have this className.

Copy link
Author

Choose a reason for hiding this comment

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

Hi Jack, yes that's good advice for me. I thought it was a small project and that it was ok to use like this, I will consider it from now on. Thank you!

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.

2 participants