Skip to content

Conversation

@Della-Bella
Copy link

Learners, PR Template

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

Briefly explain your PR.

Questions

Ask any questions you have for your reviewer.

// It is not my solution I got help with AI. I confess I need to undestand more those 3 methodos to think how to get to this.
// I will come back to it later. For now i will keep practicing more katas because they are help me.
@Della-Bella Della-Bella added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 2, 2025
@github-actions
Copy link

github-actions bot commented Nov 2, 2025

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Sprint part (Data Groups) doesn't match expected format (example: 'Sprint 2', without quotes)

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

1 similar comment
@github-actions
Copy link

github-actions bot commented Nov 2, 2025

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Sprint part (Data Groups) doesn't match expected format (example: 'Sprint 2', without quotes)

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@Della-Bella Della-Bella changed the title London | ITP-Sep-25 | Gislaine Della Bella | Data Groups | Sprint- 2 London | ITP-Sep-25 | Gislaine Della Bella | Data Groups | Sprint- 1 Nov 9, 2025
@github-actions
Copy link

github-actions bot commented Nov 9, 2025

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Sprint part (Data Groups) doesn't match expected format (example: 'Sprint 2', without quotes)

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@ckirby19 ckirby19 added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Nov 15, 2025
@github-actions
Copy link

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Sprint part (Data Groups) doesn't match expected format (example: 'Sprint 2', without quotes)

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

1 similar comment
@github-actions
Copy link

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Sprint part (Data Groups) doesn't match expected format (example: 'Sprint 2', without quotes)

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

cjyuan

This comment was marked as duplicate.

Copy link

@ckirby19 ckirby19 left a comment

Choose a reason for hiding this comment

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

Great work, mostly comments on formatting. Let me know if you are stuck on anything

// }

// Ignore this Commit
// It is not my solution I got help with AI. I confess I need to undestand more those 3 methodos to think

Choose a reason for hiding this comment

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

I appreciate your honesty :D

}

//Filter non-numeric values and create a new array of only numbers

Choose a reason for hiding this comment

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

Generally we don't leave a line between the comment and the code the comment is related to

}

//Sort the numeric list
const sortedList = [...numericList].sort((a, b) => a - b);

Choose a reason for hiding this comment

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

Can you explain why [...numericList] is needed? What would happen if I did numericList.sort instead?

Copy link
Author

Choose a reason for hiding this comment

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

because it is better not to change the original array in case I want to use it later in my code. I'm sorting a "copy" of it.

@@ -1 +1,13 @@
function dedupe() {}
function dedupe(arr) {
let result = [];

Choose a reason for hiding this comment

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

Does result have to be let here, or can it be const?
Same for item - can it be const?

Copy link
Author

Choose a reason for hiding this comment

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

I got it now. result should be const because the reference to the array doesn't need to be changed. For item in the , it can also be const, as a new item variable is declared for each iteration, because it's not reassigned within a single loop cycle.

// Given an array with no duplicates
// When passed to the dedupe function
// Then it should return a copy of the original array
test(" Given an array with no duplicates", () => {

Choose a reason for hiding this comment

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

No need for the space between " and G

// Then it should return that number
test("given an array with 1 number, it returns the number ",() => {;
const inputMax1 = [3];
const result = findMax(inputMax1);

Choose a reason for hiding this comment

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

Formatting: ensure all lines have the same indentation. Same for all tests below


let onyNumbers = theArray.filter(item => typeof item === 'number');


Choose a reason for hiding this comment

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

Formatting:

  • Misspelling of onyNumbers
  • We usually don't leave two empty lines between code like this

function sum(elements) {
function sum(theArray) {

let onyNumbers = theArray.filter(item => typeof item === 'number');

Choose a reason for hiding this comment

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

Does this need to be let?

Copy link
Author

Choose a reason for hiding this comment

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

No, It can be const because the variable is assigned once with the filtered array.

// When passed to the sum function
// Then it should return that number

test("given an array with 1 number, it returns the sam number", () => {

Choose a reason for hiding this comment

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

Misspelling: sam



function includes(list,target){
for (let value of list) {

Choose a reason for hiding this comment

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

Does this need to be let?

Copy link
Author

Choose a reason for hiding this comment

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

I start to undestand now these const and let .. i thought let would always give me less risk of error, but I can see the opposite now.

thank you for your reviwes ;)

@ckirby19 ckirby19 added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Nov 15, 2025
@github-actions
Copy link

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Sprint part (Data Groups) doesn't match expected format (example: 'Sprint 2', without quotes)

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

1 similar comment
@github-actions
Copy link

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Sprint part (Data Groups) doesn't match expected format (example: 'Sprint 2', without quotes)

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@Della-Bella
Copy link
Author

I start to undestand now these const and let .. i thought let would always give me less risk of error, but I can see the opposite now.

thank you for your reviwes ;)

@Della-Bella Della-Bella added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Nov 16, 2025
@github-actions
Copy link

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Sprint part (Data Groups) doesn't match expected format (example: 'Sprint 2', without quotes)

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

1 similar comment
@github-actions
Copy link

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Sprint part (Data Groups) doesn't match expected format (example: 'Sprint 2', without quotes)

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants