Skip to content

Manchester | 26-ITP-Sep | Salah Alsabhi | Sprint 1 | Coursework - #1482

Open
salahalsabhi wants to merge 9 commits into
CodeYourFuture:mainfrom
salahalsabhi:coursework/sprint-1
Open

salahalsabhi wants to merge 9 commits into
CodeYourFuture:mainfrom
salahalsabhi:coursework/sprint-1

Conversation

@salahalsabhi

Copy link
Copy Markdown

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

Task code

CYF-1071

Changelist

Sprint 1: fix, implement, refactor, stretch

Questions

No questions

@github-actions

This comment has been minimized.

@salahalsabhi salahalsabhi changed the title Manchester | 26-ITP-Sep | Salah Alsabhi | Sprint-1 | Coursework/sprint 1 Manchester | 26-ITP-Sep | Salah Alsabhi | Sprint-1 Nov 1, 2026
@github-actions

This comment has been minimized.

@salahalsabhi salahalsabhi changed the title Manchester | 26-ITP-Sep | Salah Alsabhi | Sprint-1 Manchester | 26-ITP-Sep | Salah Alsabhi | coursework | Sprint-1 Nov 1, 2026
@github-actions

This comment has been minimized.

@salahalsabhi salahalsabhi changed the title Manchester | 26-ITP-Sep | Salah Alsabhi | coursework | Sprint-1 Manchester | 26-ITP-Sep | Salah Alsabhi | Sprint 1 | Coursework Nov 1, 2026
@salahalsabhi salahalsabhi added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 1, 2026
@salahalsabhi

Copy link
Copy Markdown
Author

Ready for review.

Completed Sprint 1:

  • fix: calculateMedian
  • implement: findMax, sum, dedupe, mean, describeMedian
  • refactor: includes (for...of)
  • stretch: AoC 2018 day 1

All tests passing locally with npm test.

Comment thread Sprint-1/implement/dedupe.test.js Resolved
Comment thread Sprint-1/implement/max.test.js Resolved
Comment thread Sprint-1/implement/mean.test.js Resolved
Comment thread Sprint-1/implement/mean.test.js Resolved
Comment on lines +4 to +9
// Puzzle input
const changes = fs
.readFileSync(new URL("./input.txt", import.meta.url), "utf8")
.trim()
.split("\n")
.map(Number);

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.

Could you explain the purpose of calling trim() on the input before applying split('\n')?

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.

Added a short comment: trim() avoids empty lines from leading/trailing newlines that would become 0 after Number().

Comment on lines +15 to +17
for (let change of changes) {
frequency += change;
}

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.

Why not declare the loop variable change using const?

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.

Changed the loop variable to const since it is never reassigned inside the loop.

@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 Nov 4, 2026
@salahalsabhi salahalsabhi added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 5, 2026

@cjyuan cjyuan left a comment

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.

Changes look good.

Comments professionally addressed.

Excellent work.

const visited = new Set([0]);

while (true) {
for (let change of changes) {

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 missed this one.

Comment thread Sprint-1/implement/mean.test.js Resolved
@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. Reviewed Volunteer to add when completing a review with trainee action still to take. labels Nov 5, 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants