Repository navigation
Cape Town | 26-ITP-Sept | Vaidah Simenti | Sprint 3 | Refactoring Dead Code - #1727
Vaidah-dev wants to merge 2 commits into
Conversation
Vaidah-dev
added
the
Needs Review
madhuranakate
added
Review in progress
| // The sayHello function should continue to work for any reasonable input it's given. | ||
|
|
||
| let testName = "Jerry"; | ||
| const greeting = "hello"; |
There was a problem hiding this comment.
is this variable name clashing with another ?
There was a problem hiding this comment.
Oh yes, the parameter 'greeting' was shadowing the global, i have removed the global variable completely and passed "hello" directly into the clash.
| } | ||
|
|
||
| testName = "Aman"; | ||
| let testName = "Aman"; |
There was a problem hiding this comment.
Its correct that you removed the previous testName but if you are not overriding testName, is let necessary ?
There was a problem hiding this comment.
You are right, since i removed the reassignment, the value never changes so i guess 'const' is the correct choice.
madhuranakate
added
Reviewed
Vaidah-dev
added
Needs Review
There was a problem hiding this comment.
do we need to put greetings as a function parameter if its value never changes?
you can read abut parameters best practices here
KudahShambare
added
Complete
You caan also add he comments in the files you changed |
Self checklist
[X] I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
[X] My changes meet the requirements of the task
[X] I have tested my changes
[X] My changes follow the style guide
Task code
CYF-1066
Changelist
Exercise 1
Removed const greetingStr (redundant because the function built a string it never used)
Removed console.log(greetingStr) (unreachable because it is placed after return)
Removed initial testName = "Jerry" (redundant because it is overwritten before use)
Exercise 2
Removed const capitalisedPets (redundant because it is computed but never used)
Removed logPets function (unreachable because it is never called)