Repository navigation
Improve two-fer: recognise re-assignment #52
Description
Is your feature request related to a problem? Please describe.
When a solution is provided with a re-assignment, the analyzer bails out of a lot of code paths.
export function twoFer(name = 'you') {
const whomst = name
return `One for me, and one for ${whomst}.`
}When a solution is provided with a re-assignment of the named argument, the analyzer doesn't recognise the shadowing:
export function twoFer(name) {
name = name ? name : "you"
return `One for me, and one for ${name}.`
}Which exercise
two-fer
Describe the solution you'd like
Simple re-assignments like the one above should be recognised and disapproved. They don't add any value to the solution.
Re-assignments of the named arguments should be recognised and disapproved. #52 (comment)
Activity
evelynstender commentedon Dec 21, 2020
Hi @SleeplessByte,
Does this include re-assignments like:
It's re-assigning the param value. I'm asking because I caught a solution like this today.
Maybe this should also be recognised and disapproved.
Ignore me if you meant that as well haha
SleeplessByte commentedon Dec 21, 2020
evelynstender commentedon Dec 21, 2020
The id is: 73b697a598c74e3b860500d78cd45312
It suggests that you should use a default param, but I think it should be interesting saying that it's also not good to re-assign a param value.
SleeplessByte commentedon Dec 22, 2020
Thank you for providing that (it allows me to do two things: find the current message, and run this example through an offline analyzer).
Personally I'm on the fence of giving that as generic advice, but I have no problem with that advice in the context of this exercise. There is an eslint rule called
no-param-reassignthat protects against function parameter re-assignment because it also modifies theargumentsarray. It further says:I do agree with this. I think often it's by mistake. However, in this case, it'd purposeful and unambigious.
Questions for you (and anyone reading along):
argumentsarray, which might be unexpected. Use a new variable declaration instead."evelynstender commentedon Dec 23, 2020
I think it'll be interesting adding some wording like "it can cause unexpected side-effects" to make sure it's clear and maybe pointing them to The arguments object or even the eslint rule page about it.