Skip to content

Sheffield | ITP-Jan-26 | Hayriye Saricicek | Sprint 1| Module-Data-Groups#1082

Open
mshayriyesaricicek wants to merge 8 commits intoCodeYourFuture:mainfrom
mshayriyesaricicek:Sprint-1
Open

Sheffield | ITP-Jan-26 | Hayriye Saricicek | Sprint 1| Module-Data-Groups#1082
mshayriyesaricicek wants to merge 8 commits intoCodeYourFuture:mainfrom
mshayriyesaricicek:Sprint-1

Conversation

@mshayriyesaricicek
Copy link
Copy Markdown

@mshayriyesaricicek mshayriyesaricicek commented Mar 23, 2026

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

Exercises for Sprint 1

@github-actions

This comment has been minimized.

@mshayriyesaricicek mshayriyesaricicek added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 24, 2026
Copy link
Copy Markdown
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

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

The "Sprint-1" folder has a few more sub-folders containing files to be updated.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 24, 2026
@mshayriyesaricicek mshayriyesaricicek added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Mar 25, 2026
Comment on lines +19 to +20
describe("findMax.js", () => {
test.each([
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should you need to classify test data into different categories, you can write:

describe("findMax -- description of category 1", () => {
  test.each(
    ...
  );
});

describe("findMax -- description of category 2", () => {
  test.each(
    ...
  );
});
...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

thank you

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 25, 2026
@mshayriyesaricicek mshayriyesaricicek added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Apr 2, 2026
Comment on lines +18 to +21
if (!foundValid) {
//if no valid numbers found, returns"-Inifinity" this includes invalid data and empty arrays
return -Infinity;
}
Copy link
Copy Markdown
Contributor

@cjyuan cjyuan Apr 3, 2026

Choose a reason for hiding this comment

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

If foundValid remains false, then max would remain unchanged. So even without using foundValid and without this if statement and, the function would still return -Infinity.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

thank you for explaining

const result = sum(input);

if (typeof expected === "number") {
expect(Math.abs(result - expected)).toBeLessThan(1e-10); // for special cases of decimals to avoid precision issues
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can also use expect(result).toBeCloseTo(expected).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

thank you, that is useful to know

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: They are not entirely equivalent though.

expect(result).toBeCloseTo(expected, 10) is closer to (Math.abs(result - expected)).toBeLessThan(0.5e-10)

The default value of the second parameter is 2 (surprisingly). So

expect(result).toBeCloseTo(expected) is closer to (Math.abs(result - expected)).toBeLessThan(0.005)
(Note: 0.5e-2 = 0.5 * 10**-2 = 0.5 * 0.01 = 0.005)

total += val; //adds number to sum
}

return foundValid ? total : "Invalid Input"; // if at least one valid value is found return sum if no valid number is found return Invalid Input
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What do you think about designing the function to support this behavior?
sum(["A"]) + sum([2]) == sum(["A", 2])

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I don't understand what it is you want me to do. The idea of the function is to add numbers together and ignore everything else so I am not sure why you are asking me to do this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sum(array1) + sum(array2) == sum(array1.concat(array2))

Both sides are two different ways to compute "sum of all the elements in array1 and array2".
Normal expectation is that the equality should hold.

However, if array1 is ["A"] and sum(array1) returns "Invalid Input" instead of 0, then the equality would break.

As a result, in my opinion, 0 is a more appropriate value to return when an array contains only non-numeric values.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Apr 3, 2026
@mshayriyesaricicek mshayriyesaricicek added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Data-Groups The name of the module. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Apr 4, 2026
@cjyuan
Copy link
Copy Markdown
Contributor

cjyuan commented Apr 4, 2026

Did you make any new commits? If you did, don't forget to push them to GitHub too. I will mark this PR as Complete first.

@cjyuan cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Apr 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Module-Data-Groups The name of the module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants