Skip to content

Conversation

@M-Abdoon
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

I've done the Sprint 1 exercises. including fixing, implementing and refactoring code.

Questions

No questions so far, thank you.

@M-Abdoon M-Abdoon added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 24, 2025
@AbogeJr AbogeJr 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 3, 2025
Copy link

@AbogeJr AbogeJr left a comment

Choose a reason for hiding this comment

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

Good work overall. Please check out the comments and resubmit for review.

}
}
return false;
const includes = (list, target) =>{
Copy link

Choose a reason for hiding this comment

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

While this may work in most JS environments, could you double check the for...of syntax and see if ther is a better way to write this.

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. I didn't declare the variable 'item' properly. I should have put 'let' or 'const' before the variable. It works, but the code fails when running in strict mode ( "use strict"; ). It is safer to declare it the right way. I updated the code so the for..of syntax is better and safer.

return null;

// filter out non-numeric values inside the list
list = list.filter(item => Number.isInteger(item));
Copy link

Choose a reason for hiding this comment

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

Good stuff. You’re currently using Number.isInteger() to check numbers. Could there be a better way to include floating-point numbers in your median calculation too?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, will be better to use Number.isFinite() instead of Number.isInteger if we want to include floating-point numbers.
I updated filter() so floating-point numbers are included.

@@ -1 +1,7 @@
function dedupe() {}
function dedupe(arr) {
arr = new Set(arr); // Using `new Set()` to store only unique values in the same variable
Copy link

Choose a reason for hiding this comment

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

Good use use of Set.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you ^^

@AbogeJr AbogeJr 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 3, 2025
@M-Abdoon
Copy link
Author

M-Abdoon commented Nov 4, 2025

Good work overall. Please check out the comments and resubmit for review.

Thank you so much for the review. I updated the code and made changes.

@M-Abdoon M-Abdoon 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 Nov 4, 2025
@github-actions
Copy link

github-actions bot commented Nov 4, 2025

Your PR couldn't be matched to an assignment in this module.

Please check its title is in the correct format, and that you only have one PR per assignment.

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).

@AbogeJr AbogeJr 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 4, 2025
@AbogeJr
Copy link

AbogeJr commented Nov 4, 2025

Nice work with the changes👍🏾. There might be a small issue with your PR title though. Please check it out and make sure it strictly follows the format. After that you should be all done.

@AbogeJr AbogeJr 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 4, 2025
@M-Abdoon M-Abdoon changed the title Glasgow | 25-ITP-SEP | Mohammed Abdoon | Sprint 1| Coursework Glasgow | 25-ITP-SEP | Mohammed Abdoon | Sprint 1| Sprint-1 Nov 5, 2025
@M-Abdoon M-Abdoon 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 Nov 5, 2025
@M-Abdoon
Copy link
Author

M-Abdoon commented Nov 5, 2025

Nice work with the changes👍🏾. There might be a small issue with your PR title though. Please check it out and make sure it strictly follows the format. After that you should be all done.

Thank you so much. I updated the PR's title.

@AbogeJr AbogeJr 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 Nov 5, 2025
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.

2 participants