Skip to content

London | 26-ITP-SEP | Russom Gebremeskel | Sprint 3 | Dead Code - #1632

Open
russom-g wants to merge 30 commits into
CodeYourFuture:mainfrom
russom-g:sprint-1/dead-code
Open

russom-g wants to merge 30 commits into
CodeYourFuture:mainfrom
russom-g:sprint-1/dead-code

Conversation

@russom-g

@russom-g russom-g commented Sep 30, 2026 •

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-1066

Changelist

Unused code remove. Function refactored to continue to work for any input it's given.

@github-actions

This comment has been minimized.

@russom-g russom-g added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 3, 2026
@github-actions

This comment has been minimized.

@github-actions github-actions Bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 3, 2026
@russom-g russom-g added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 5, 2026

@abdishakoor-dev abdishakoor-dev 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.

You found all the real dead code: greetingStr and the line after return in exercise 1, and capitalisedPets and logPets in exercise 2. The trouble is that more than the dead code has gone, so neither file prints what it did before.

Before I can mark this Complete:

  1. exercise-1.js now prints nothing. The starter printed hello, Aman!, and that final log is what the dead code was being removed around. See my comment on line 6.
  2. exercise-2.js should still print { HAMSTER: 3, HORSE: 1 } from the pets array, and countAndCapitalisePets should still count whatever array it is given. See my comments on lines 4, 10 and 25.
  3. PR tidy-up: add CYF-1066 under a "Task code" heading in the description, update the title to London | 26-ITP-Sep | Russom Gebremeskel | Sprint 3 | Dead Code (you're in the September cohort now, and this task is Sprint 3), and remove the NotCoursework label. That label tells volunteers the PR isn't coursework and to skip it; the bot's message was asking for the task code, not that label.

Add the Needs Review label again once you've pushed.

Comment thread dead-code/exercise-1.js
const greetingStr = greeting + ", " + name + "!";
return `${greeting}, ${name}!`;
console.log(greetingStr);
}

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.

Run node dead-code/exercise-1.js on main and on your branch. What does each print? Dead code is code that never affects the output, so was console.log(greetingMessage) dead?

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 made the sayHello function to continue to work for any reasonable input it's given.

I added a console.log() now

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.

The log is back, thanks. Run node dead-code/exercise-1.js now: it prints Hi, Aman!, but main prints hello, Aman!. Where does the Hi on line 8 come from, and what did the starter pass in as the greeting?

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 fixed the problem

Comment thread dead-code/exercise-2.js
@@ -2,27 +2,24 @@
// The countAndCapitalisePets function should continue to work for any reasonable input it's given, and you shouldn't modify the pets variable.

const pets = ["parrot", "hamster", "horse", "dog", "hamster", "cat", "hamster"];

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.

Where is pets used now?

@russom-g russom-g Oct 22, 2026 •

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 misunderstood the instruction. It is fixed now.

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.

Fixed, pets is used again and unchanged.

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.

Done.

Comment thread dead-code/exercise-2.js Outdated
}
});
petsArr
.filter((pet) => pet[0] === "h")

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.

The comment on line 2 says this function should keep working for any reasonable input. What does countAndCapitalisePets(["dog", "cat", "dog"]) return on main, and what does it return now?

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.

Updated.

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.

Try it: countAndCapitalisePets(["dog", "cat", "dog"]) returns {} with your version. On main it returns { DOG: 2, CAT: 1 }.

The .filter((pet) => pet[0] === "h") on line 10 is the reason. In the starter, the filtering happened outside the function, on its own line: const petsStartingWithH = pets.filter(...). That line wasn't dead code, because its result was what got passed into the function.

Put that line back under pets, take the filter out of the function so it goes back to a plain petsArr.forEach(...), and pass petsStartingWithH into the function on the last line. Your counting code on lines 12 to 18 doesn't need to change.

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.

Done.

Comment thread dead-code/exercise-2.js Outdated

console.log(countedPetsStartingWithH); // { 'HAMSTER': 3, 'HORSE': 1 } <- Final console log
const animals = ["lion", "hyena", "elephant", "hippo", "tiger", "hippo"];
console.log(countAndCapitalisePets(animals));

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.

The starter's final log printed { HAMSTER: 3, HORSE: 1 }. What does yours print, and which array is it counting?

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.

Updated.

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.

The output is right again. Once the filter moves back out of the function (see line 10), make sure it still prints { HAMSTER: 3, HORSE: 1 }.

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.

Done.

Thanks for the feedback @abdishakoor-dev

@abdishakoor-dev abdishakoor-dev 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 Oct 5, 2026
@russom-g russom-g changed the title London | 26 ITP MAY | Russom Gebremeskel | Sprint 1 | Dead Code London | 26 ITP SEP | Russom Gebremeskel | Sprint 1 | Dead Code Oct 13, 2026
@russom-g russom-g changed the title London | 26 ITP SEP | Russom Gebremeskel | Sprint 1 | Dead Code London | 26 ITP SEP | Russom Gebremeskel | Sprint 3 | Dead Code Oct 22, 2026
@russom-g russom-g 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. NotCoursework labels Oct 22, 2026
@russom-g russom-g changed the title London | 26 ITP SEP | Russom Gebremeskel | Sprint 3 | Dead Code London | 26-ITP-SEP | Russom Gebremeskel | Sprint 3 | Dead Code Oct 22, 2026

@abdishakoor-dev abdishakoor-dev 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.

Thanks for replying to each comment. The PR tidy-up is done too: title, task code and label are all sorted, and exercise 2 prints { HAMSTER: 3, HORSE: 1 } again.

Three things left before I can mark this Complete:

  1. exercise-2.js: countAndCapitalisePets only counts pets that start with "h". The filter is now inside the function (line 10), so any other array gives the wrong answer. My reply on line 10 explains how to move it back out.
  2. exercise-1.js prints Hi, Aman!. The starter printed hello, Aman!, and removing dead code shouldn't change the output. See my reply on line 6.
  3. Prettier on exercise-1.js. The file is missing the empty line at the end. Right click in the editor, choose Format Document, and save. To make this happen every time you save, follow the format on save steps here: https://github.com/CodeYourFuture/Module-JavaScript-Fundamentals/blob/main/practical_guide.md

Run node dead-code/exercise-1.js and node dead-code/exercise-2.js before you push and check they print exactly what main prints. Add the Needs Review label again once you've pushed.

@abdishakoor-dev abdishakoor-dev 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 Oct 29, 2026
@russom-g russom-g 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 Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants