Skip to content

London | 26-ITP-Sep | Mars Adesina | Sprint 3 | Refactoring Dead Code #1745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
marscancode wants to merge 2 commits into
base: main
Choose a base branch
from
Open

London | 26-ITP-Sep | Mars Adesina | Sprint 3 | Refactoring Dead Code #1745

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ce316cc
Removed dead code from sayHello function
marscancode Oct 21, 2026
4afd79c
removed dead code from the countCapitalisePets function
marscancode Oct 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions dead-code/exercise-1.js
View file
Open in desktop
Comment thread
marscancode marked this conversation as resolved.
Resolved
Original file line number Diff line number Diff line change
@@ -1,17 +1,8 @@
// Find the instances of unreachable and redundant code - remove them!
// The sayHello function should continue to work for any reasonable input it's given.

let testName = "Jerry";
const greeting = "hello";

function sayHello(greeting, name) {
const greetingStr = greeting + ", " + name + "!";
return `${greeting}, ${name}!`;
console.log(greetingStr);
}

testName = "Aman";

const greetingMessage = sayHello(greeting, testName);

console.log(greetingMessage); // 'hello, Aman!'
console.log(sayHello("hello", "Aman"));
8 changes: 1 addition & 7 deletions dead-code/exercise-2.js
View file
Open in desktop

@iscmiguelsamaniego iscmiguelsamaniego Oct 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Excellent work start on structuring the function and array operations.

Quick question:

countedPetsStartingWithH is declared twice, and the function currently returns an empty object. How might you iterate through the array to populate the frequency map?

@marscancode marscancode Oct 23, 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.

Hi , Thanks for reviewing my work. I'm a little unsure about your feedback regarding the duplicate declaration and the empty object. I moved the countedPetsStartingWithH declaration rather than keeping both declarations, and my function already had the .forEach() loop to populate petCount. When I run my current code, it returns { HAMSTER: 3, HORSE: 1 }. Could you clarify what I need to change or which part I may have misunderstood? Thanks!

@dkrest1 dkrest1 Nov 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hi @marscancode

Just following up on your question from the earlier review.

I've checked the code currently shown in this PR, and countAndCapitalisePets() appears to initialise petCount as an empty object and return it without populating it.

You mentioned having a .forEach() loop that produces { HAMSTER: 3, HORSE: 1 }, but I can't see that loop in the version currently available here.

Could you check whether your latest changes have been committed and pushed to this branch?

That might explain the difference between what you're seeing locally and what's showing in the PR.

Thanks for following up on the feedback!

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

Hi, thanks for following up!

I've checked this again and confirmed that the changes are committed and pushed to coursework/sprint-3-dead-code.

I've also checked commit 4afd79c directly, and it contains the .forEach() loop that populates petCount, followed by return petCount.

The current version of the file on GitHub also shows this implementation:

https://github.com/marscancode/Module-Structuring-and-Testing-Data/blob/coursework/sprint-3-dead-code/dead-code/exercise-2.js

Could you have another look when you get a chance? I'd appreciate it if you could let me know whether there's anything else I've misunderstood.

Thanks!

Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,8 @@
// 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"];
const capitalisedPets = pets.map((pet) => pet.toUpperCase());
const petsStartingWithH = pets.filter((pet) => pet[0] === "h");

function logPets(petsArr) {
petsArr.forEach((pet) => console.log(pet));
}
const countedPetsStartingWithH = countAndCapitalisePets(petsStartingWithH);

function countAndCapitalisePets(petsArr) {
const petCount = {};

@dkrest1 dkrest1 Nov 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hi Mars! 👋

Thanks for clarifying your approach in the earlier discussion.

Looking at the current version of exercise-2.js, I can see that petCount is initialised as an empty object and returned without being populated.

You mentioned using .forEach() to count the pets, but I couldn't find that loop in the current code.

Could you double-check whether your latest changes have been committed and pushed to this PR?

The expected output is { HAMSTER: 3, HORSE: 1 }, but the current implementation appears to return {}.

Keep up the good work!

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

Hey,

I've checked my latest commit (4afd79c) and confirmed that the .forEach() loop is present in the committed version of exercise-2.js.

I wonder whether the unchanged lines are being collapsed in the PR diff, as the loop was already part of my original implementation and wasn't changed in this commit.

I've checked the full file on GitHub, and the function initialises petCount, populates it using .forEach(), and then returns it.

Here is a direct link to the complete file:
https://github.com/marscancode/Module-Structuring-and-Testing-Data/blob/coursework/sprint-3-dead-code/dead-code/exercise-2.js

Thanks for the feedback and hopefully this clears things up but let me know if not.

Expand All @@ -23,6 +19,4 @@ function countAndCapitalisePets(petsArr) {
return petCount;
}

const countedPetsStartingWithH = countAndCapitalisePets(petsStartingWithH);

console.log(countedPetsStartingWithH); // { 'HAMSTER': 3, 'HORSE': 1 } <- Final console log
Loading