Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Good refactoring vs. bad refactoring

builder.io

Post reply on HN
Print view View original

81–90 of 154 posts

Re: Good refactoring vs. bad refactoring

#81

Post by nine_k »

Joker_vD wrote: post #73
Jean-Papoulos wrote: post #40

Earlier quoted context omitted.

That's because they are. Functional code is more readable. And if you look back, basically all advances in programming languages have been about "making stuff more readable". Thus, for loops (for this usage) are "old".

> Functional code is more readable. There is no way that name: R.pipe(R.prop('name'), R.toUpper), age: R.prop('age'), isAdult: R.always(true) is more readable than name: user.name.toUpperCase(), age: user.age, isAdult: true

Certainly so! The first example is needlessly contrived.

But instead of

  for (const i=0; i
you can write

  const new_data = old_data.map((x) => x.toUpperCase());
I think it's both more clear and less error-prone.

Re: Good refactoring vs. bad refactoring

#82

Post by persnickety »

The_Colonel wrote: post #14
p2501 wrote: post #4

Earlier quoted context omitted.

Aesthetic aside, I am under the impression that people start programming, by and large, with imperative for/if style => so the imperative style is readable by more people. Even for more experienced programmers, reading imperative probably cost less energy, since it is more internalised? Futhermore, in JS, the functionnal style is less performant (nearly twice on my machine, i assume because it do less useless memory…

> Even for more experienced programmers, reading imperative probably cost less energy, since it is more internalised? I disagree. For-cycles are usually more difficult to reason about, because they're more general and powerful. If I see "for (...", I only know that the subsequent code will iterate, but the actual meaning has to be inferred from the content. Meanwhile, a .map() or .filter() already give me hints - the…

> If I see "for (...", I only know that the subsequent code will iterate

And then someone slaps do {} while(0) in a macro.

Re: Good refactoring vs. bad refactoring

#83

Post by vips7L »

Terr_ wrote: post #80
vips7L wrote: post #75

Earlier quoted context omitted.

Yes this is just extreme bike-shedding at this point. But none of this is impossible with more OO principles, like interfaces: class User { // Convenience function to check if the user is an adult in their current location boolean isAdult() { return this.location.isAdult(this); } boolean isOfDrinkingAge() { return this.location.isOfDrinkingAge(this); } } interface Location { boolean isAdult(User u); boolean isOfDrink…

That feels like unnecessary levels of indirection to provide a method that shouldn't be on the User anyway. j = Jurisdiction.fromUserLocation(user); j.isOfDrinkingAge(user);

> that shouldn't be on the User anyway.

That’s just your opinion. It’s ok to provide convenience functions. I see no difference between the amount of indirection in our implementations, except mine is in the more natural place and you don’t have to know how to get a location or jurisdiction to answer the question: “is this user an adult?”. Knowing that it uses a location or jurisdiction is an implementation detail that you shouldn’t couple yourself to.

Cheers mate, I think I’m done moving goal posts for this conversation :)

Re: Good refactoring vs. bad refactoring

#84

Post by _a_a_a_ »

Y_Y wrote: post #59
MattHeard wrote: post #45

I got as far as here: > If you need to introduce a new pattern, consider refactoring the entire codebase to use this new pattern, rather than creating one-off inconsistencies. Putting aside the mis-application of "pattern" (which _should_ be used with respect to a specific design problem, per the Gang of Four), this suggestion to "refactor the entire codebase" is impractical and calcifying. Consistency increases legi…

> "pattern" (which _should_ be used with respect to a specific design problem, per the Gang of Four) Why is that true? Particularly if you're not an OOP user/believer. It's not like "pattern" is some obscure term of art.

For consistency, the same design problems should have the same design solutions a.k.a. pattern. If you don't value consistency, feel free to take a different approach every time. That will confuse your users. I used to work with a guy, the simple problem of reading a CSV was done using a library, problem sorted. Out of sheer excitement he then rewrote it was with combinator parsers, then as some astronaut architect functional monstrosity, so complex no one else could comprehend it.

This is how not to do it – for same problem, use same solution. I admit that's an extreme case but it's also a real one and illustrates the issue well.

(also patterns are not specific to OO, nor is OO incompatible with a functional style)

Re: Good refactoring vs. bad refactoring

#85

Post by nuancebydefault »

boxed wrote: post #62
watwut wrote: post #22

Earlier quoted context omitted.

I think that was the point - the library added nothing, the same thing could be done with pure javascript.

If that was the point, then that paragraph needs to be rewritten badly.

In fact the paragraph needs _good_ refactoring

Re: Good refactoring vs. bad refactoring

#86

Post by Flop7331 »

charlie0 wrote: post #78

Agreed with everything except the following: >Remember, consistency in your codebase is key. If you need to introduce a new pattern, consider refactoring the entire codebase to use this new pattern, rather than creating one-off inconsistencies. It's often times not practical (or even allowed by management due to "time constraints") to refactor a pattern out of an entire codebase if it's large enough. New patterns can…

The key word is consider. If you wouldn't apply the pattern to the whole codebase, maybe you don't actually want to introduce it in just this one new place.

Re: Good refactoring vs. bad refactoring

#87

Post by nuancebydefault »

nine_k wrote: post #81
Joker_vD wrote: post #73

Earlier quoted context omitted.

> Functional code is more readable. There is no way that name: R.pipe(R.prop('name'), R.toUpper), age: R.prop('age'), isAdult: R.always(true) is more readable than name: user.name.toUpperCase(), age: user.age, isAdult: true

Certainly so! The first example is needlessly contrived. But instead of for (const i=0; i you can write const new_data = old_data.map((x) => x.toUpperCase()); I think it's both more clear and less error-prone.

The second code does not compile and introduces a new dependency.

Re: Good refactoring vs. bad refactoring

#88

Post by doctorM »

Reading this I realised I've kind of drifted away from the idea of refactoring for the point of it.

The example with the for-loop vs. map/filter in particular - it's such a micro-function that whichever the original author chose is probably fine. (And I would be suspicious of a developer who claimed that one is 'objectively' better than the other in a codebase that doesn't have an established style one way or the other).

Refactor when you need to when adding new features if you can reuse other work, and when doing so try to make minimal changes! Otherwise it kind of seems more like a matter of your taste at the time.

There's a limit of course, but it's usually when it's extremely obvious - e.g. looong functions and functions with too many parameters are obvious candidates. Even then I'd say only touch it when you're adding new features - it should have been caught in code review in the first place, and proactively refactoring seems like a potential waste of time if the code isn't touched again.

The (over) consolidation of duplicated code example was probably the most appealing refactor for me.

Re: Good refactoring vs. bad refactoring

#89

Post by nuancebydefault »

kolme wrote: post #44
jv_be wrote: post #28

A good refactor does not change behaviour, I would like the author to start with that point. Take many more much smaller steps while doing so. Not touching a piece of code in the first 6 to 9 months is something I don’t really agree with. Breaking complex methods up by extracting variables and methods can really help learning the code, whilst not breaking it. If you are worried about consistency, just pair up of prac…

A refactor does not change behavior, period . By definition. If something changed, it's not a refactor. It's a change. Like in the example where the caching was removed: NOT a refactor. Ore where the timeouts for the requests were changed: NOT a refactor. The definition of refactor: change the structure of the code without altering the behavior. It's like saying a crash is a bad landing.

By _your_ definition. If you apply it, a lot of effort is needed for little gain. The best refactors i've seen improved behavior while making code more concise.

Re: Good refactoring vs. bad refactoring

#90

Post by bobthepanda »

carlmr wrote: post #27
boxed wrote: post #15

Earlier quoted context omitted.

The dig against FP is weird since the "good refactor" also uses FP, just a built in one in JS. Which I agree is better, but mostly by being built in and idiomatic, it's still exactly as functional.

On point. Even then mentioning it used filter and map. But the bad refactor also uses filter and map. It's the exact same change of programming paradigm. Given the text, I would have expected some minor refactor with range-based for loops (are these a thing? My JS is rusty). Where you get the advantage of map (no off-by-one indexing errors) without changing the programming paradigm.

you don't even need range loops.

you have forEach:

   list.forEach((item, index) => doSomething());
you also have for/of, though that doesn't have an index if you need that

   for (const item of list) { doSomething() };
which just obviates the need for index incrementing in the most common case, where you are incrementing by one until you hit the end of the list.
Post reply on HN