London | ITP-Sep-25 | Gislaine Della Bella | Data Groups | Sprint- 1#810
London | ITP-Sep-25 | Gislaine Della Bella | Data Groups | Sprint- 1#810Della-Bella wants to merge 5 commits intoCodeYourFuture:mainfrom
Conversation
// 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.
|
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
|
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). |
|
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). |
|
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
|
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
left a comment
There was a problem hiding this comment.
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 |
| } | ||
|
|
||
| //Filter non-numeric values and create a new array of only numbers | ||
|
|
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
Can you explain why [...numericList] is needed? What would happen if I did numericList.sort instead?
There was a problem hiding this comment.
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 = []; | |||
There was a problem hiding this comment.
Does result have to be let here, or can it be const?
Same for item - can it be const?
There was a problem hiding this comment.
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", () => { |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
Formatting: ensure all lines have the same indentation. Same for all tests below
|
|
||
| let onyNumbers = theArray.filter(item => typeof item === 'number'); | ||
|
|
||
|
|
There was a problem hiding this comment.
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'); |
There was a problem hiding this comment.
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", () => { |
|
|
||
|
|
||
| function includes(list,target){ | ||
| for (let value of list) { |
There was a problem hiding this comment.
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 ;)
|
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
|
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). |
|
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 ;) |
|
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
|
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). |
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.