Refactor code correctly

#typescript #javascript

[koushikseal]

Why use filter + map making the code loop twice when you have reduce?
For performance optimization when the input array is big, basic loops are preferred.

[rranon]

Nice example! I was quite okay with the first version and then you showed how you would refactor it and it definitely looked better!

[prashantsawant]

While improving its appearance, the performance was compromised. Initially, the filtering makes one pass through the list, and then the mapping makes another.

[multigladiator]

everyone should know how to use forEach, filter and map...
I consider this as basics.
Moreover, one simple for loop could satisfy both requirements here. 1n vs 2n complexity wise. This can make a difference of seconds if you have large iterables

[ShortFilmVD]

Efficiency aside, extracting logic into individual, well named functions will make any code far easier to read and reason about.

In this example, you could make a function called something like "filterUsersOverAge(age)" for the filtering and another function, say "makeAdultUser(user)", for the mapping.

This reduces the responsibility of the main function, making softer (easier to change and therefore maintain, and extend). Soft software is good 😉

I'd also suggest renaming the "processUsers" function to reflect what it's actually doing e.g. "filterAdultUsers", but I appreciate that naming isn't the point of this example.

Happy coding 👍

[goldstein]

Ok, thats plan
1) format user is separate function
2) use filter && map if there re small count of users (less than 100k, ex)
3) rename function - so bad name now

[apana]

I literally just had a heart attack seeing that code.

[janekschleicher]

More critical for this code example is that it's unclear why username should be upper cased and why hardcoded an age as filter. At best, both would be configurable, and also a comment is needed why these format/filter setting ate in this way.
The namings used in the example are definitely not explaining well, the function us called process (what could mean anything) and usually I'd expect a function called process that it might change its inputs or have other intentional side effects, but the result variable is called formatted. I guess a name like getAdultsFormatted or something similar might be much better function name, and IMHO more important to refactor than a for loop to a map reduce functinal style. Renaming the function would remove friction when reading the source code where it's getting called.
I'd prefer the functional style, but there are enough outside who don't like the extra abstractions and wouldn't know how to debug it in case, so usually if I'd found code in the original for loop over i style, I would keep it untouched, as it's a strong indicator, this is the situation here in the team, especially if the dev who wrote it is a senior.
In a code review for a junior, I'd show him the slightly cleaner functional way, but as said, I think this is just style, not major reasoning for refactor. And such a change always means a need to unit test the change as it is possible tointroduce a subtle bug here.

[Applecitylightkiwi]

Hmm so you use js methods instead of raw for loop ?

[pyroandi]

There is a 0% chance a dev would refactor that this way 😂

[Chavezd]

I find the “I hired a…” segment tasteless. You could provide an example without that statement

[journeytocoding]

What is refactoring anyway? Please tell