DEV Community

loading...

Discussion on: Refactoring the Worst Code I’ve Ever Written

Collapse
jnschrag profile image
Jacque Schrag Author

Hey, thanks for the comments! I'm always happy to hear constructive criticism on how to make my code better. :)

  1. If I'm understanding you correctly, you're suggesting creating the arrays of types, years, and regions within the .reduce function? That's not a bad idea, and you're right, would provide some optimization. I didn't explain this in the post, but part of my rationale for keeping it separate is to maintain separation of concerns. Those 3 lines are specifically about isolating the values of those variables within the dataset, while the .reduce accomplishes a separate task. But you make a good point and it's something to consider for future implementations!

  2. I hadn't considered storing those values as a Set instead of an array, that's a great idea

  3. Yep! However, I intentionally wrote the longer version for clarity. :) Certainly is worth considering how to make it more concise in production, though!

Collapse
fardarter profile image
sauln

Re 1, it's not so much where it is but rather that I suspect Array.from loops and new Set loops and the map loops, which means 3 loops for each construction of those items.

That said, you could do it all in one reduce and keep the concerns separated with object keys, and then destructure them into variables in the outer scope.

I think re 3 you're talking about my non-mutation comment?

If you destructure the variables it can be terser, but my case is more for non-destructive operations. I don't like modifying things because it makes the original unavailable for later use without larger refactor. But again I'm an ideologue about this and it isn't common opinion.

Forem Open with the Forem app