Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
1. Replace `<your_account>` with your Github username in the link
- [DEMO LINK](https://<your_account>.github.io/js_get_data_DOM/)
- [DEMO LINK](https://TiFAu.github.io/js_get_data_DOM/)
2. Follow [this instructions](https://mate-academy.github.io/layout_task-guideline/)
- There are no tests for this task so use `npm run lint` command instead of `npm test`

Expand Down
33 changes: 32 additions & 1 deletion src/scripts/main.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,34 @@
'use strict';

// write your code here
const spanPopulation = [...document.querySelectorAll('.population')];

Choose a reason for hiding this comment

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

Selecting the elements this way is okay, but when you later read values prefer textContent (and .trim()) instead of innerHTML to get raw text without accidental HTML or extra whitespace.

const averagePopulation = document.querySelector('.average-population');

Choose a reason for hiding this comment

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

No null-check for averagePopulation. If the element is missing document.querySelector returns null and later assignment will throw — guard before writing to it.

const totalPopulation = document.querySelector('.total-population');

Choose a reason for hiding this comment

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

No null-check for totalPopulation. Add an existence check before assigning to avoid runtime errors when element is missing.

const total = spanPopulation.reduce(

Choose a reason for hiding this comment

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

This reduce sums every span value without validating conversion to a finite number. Checklist #3 requires verifying strings are convertible before using them — instead, build an array of parsed numbers (parse + filter with Number.isFinite) and reduce that array.

(sum, n) => sum + Number(n.textContent.trim().split(',').join('')),

Choose a reason for hiding this comment

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

Cleaning here uses split/join to remove commas. Use a single replace to remove commas and whitespace (including NBSP) before Number(), e.g. .replace(/[,\u00A0\s]/g, ''), then convert and check Number.isFinite(...) (checklist #3).

0,
);
Comment on lines 6 to 9

Choose a reason for hiding this comment

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

Using reduce with Number(n.innerHTML.split(',').join('')) will produce NaN if any value is not a valid number and then the whole total becomes NaN. Instead, clean each span's text (use textContent.trim()), remove separators (e.g. .replace(/[,\s]/g, '')), parse with Number or parseInt, check !Number.isNaN(value)/Number.isFinite(value), collect valid numbers, and reduce over that array.

Comment on lines 6 to 9

Choose a reason for hiding this comment

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

The reduce-based summation here uses Number(n.innerHTML.split(',').join('')) directly. If any span contains non-numeric text this will produce NaN. Per the requirements you must ensure the string can be converted to a number and filter out invalid entries before summing — e.g. build validNumbers by reading n.textContent.trim(), removing separators, converting with Number(...) and keeping only values where Number.isFinite(value), then reduce over validNumbers.

const average = Math.round(total / spanPopulation.length);

Choose a reason for hiding this comment

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

Average is computed as total / spanPopulation.length. The requirement says to calculate based on parsed numbers — the denominator should be the count of successfully parsed numeric values, and you must guard against division by zero (empty or all-invalid inputs).

Choose a reason for hiding this comment

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

Average is computed as Math.round(total / spanPopulation.length). The requirement is to compute the average based on the count of successfully parsed numeric values (not the original node list). Compute average using the length of the filtered validNumbers array and guard against division by zero (e.g. show 0 or keep 'Calculate it!' if there are no valid numbers).

Choose a reason for hiding this comment

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

Average is calculated using spanPopulation.length. The average must be based on the count of valid parsed numbers (not total spans). Also guard against division by zero when there are no valid numbers (checklist #4).


function convertNumbers(number) {

Choose a reason for hiding this comment

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

convertNumbers assumes it receives a valid finite number; if you pass NaN or Infinity the output will be wrong. Ensure inputs are validated before calling this function (and round the average before formatting as required).

Choose a reason for hiding this comment

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

convertNumbers assumes it receives a valid finite integer. Ensure you only call it with validated finite numbers (round the average first). Alternatively consider Intl.NumberFormat for thousands separators.

const resultNumber = [];
const numberToString = String(number);

Choose a reason for hiding this comment

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

convertNumbers assumes the input is a valid integer string; if number is NaN or not an integer the output will be wrong. Ensuring earlier validation (only calling this with finite integers) will fix this — or consider using Intl.NumberFormat for robustness.


for (let i = numberToString.length; i > 0; i -= 3) {
if (i < 3) {
resultNumber.unshift(numberToString.slice(0, i));

break;
}
resultNumber.unshift(numberToString.slice(i - 3, i));
}

return resultNumber.join(',');
}

if (averagePopulation) {
averagePopulation.textContent = convertNumbers(average);

Choose a reason for hiding this comment

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

When writing results to the DOM you currently overwrite unconditionally. Check that the computed average is finite before calling convertNumbers and setting textContent — otherwise you may replace 'Calculate it!' with NaN or Infinity. The same applies to total (checklist #5/#6).

}

if (totalPopulation) {
totalPopulation.textContent = convertNumbers(total);

Choose a reason for hiding this comment

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

Also check the total before formatting/assigning: only call convertNumbers(total) if Number.isFinite(total) (or compute total from the validated array as suggested).

}
2 changes: 1 addition & 1 deletion src/styles/_fonts.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
@font-face {
font-family: Roboto, Arial, Helvetica, sans-serif;
src: url('../fonts/Roboto-Regular-webfont.woff') format('woff');
font-weight: normal;
font-style: normal;
src: url('../fonts/Roboto-Regular-webfont.woff') format('woff');

Choose a reason for hiding this comment

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

Make sure the font file exists at the given relative path; if the file is missing the @font-face rule will fail and the custom font won't load.

}
38 changes: 23 additions & 15 deletions src/styles/main.scss
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
@import 'fonts';

body {
background: #eee;
counter-reset: section;

display: flex;

Choose a reason for hiding this comment

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

The task requires changing only main.js and not styles or HTML (checklist items #9 and #10). This stylesheet appears to have been modified in the submission — please revert any changes to src/styles/main.scss so only main.js is changed.

margin: 0;
align-items: center;
justify-content: center;

width: 100vw;
height: 100vh;
justify-content: center;
align-items: center;
margin: 0;

font-family: Roboto, sans-serif;
counter-reset: section;

background: #eee;
}

h1 {
Expand All @@ -18,28 +22,32 @@ h1 {

.list {
margin: 32px 0;

Choose a reason for hiding this comment

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

Further style rules were edited here (e.g. list and spacing). Reverting the entire styles folder (or this file) to the original will ensure the submission follows the task constraint to modify only main.js.

list-style: none;
padding: 0;
list-style: none;
&__item {
margin: 8px;
border-bottom: 1px solid darkgrey;
position: relative;

display: flex;
justify-content: space-between;
padding: 4px 4px 4px 30px;
position: relative;

img {
margin-right: 8px;
}
margin: 8px;
padding: 4px 4px 4px 30px;
border-bottom: 1px solid darkgrey;

&::before {
color: #565754;
counter-increment: section;
content: counter(section);
counter-increment: section;

position: absolute;
top: 50%;
left: 5px;
transform: translateY(-60%);

color: #565754;
}

img {
margin-right: 8px;
}
}
}
Expand Down
Loading