4 ms·
Decreasing code duplication is good of course. But you need to be careful that the two pieces of code are inherently equal and not just by accident. Because onc
by wereHamster 9y ago
Decreasing code duplication is good of course. But you need to be careful that the two pieces of code are inherently equal and not just by accident. Because once you consolidate the code, for example like you suggest, and somebody comes along and changes `GOOD_NAME_FOR_MAGIC_CODE` to something else, without being aware that it is used in multiple places, he might inadvertently make a change he did not mean to do. Tests help, a good IDE helps (to find location where a symbol is used). But still, it's a fine line to walk.
- chairmanwow 9y agoWouldn't this name change be immediately caught by the compiler?
- dankent 9y agoI assume wereHamster is referring to the value of the constant being changed, rather than the name.
- koolba 9y agoI'd bet dollars to donuts the situations where you want it to change outnumber the ones where you don't by more than 10:1.
- Jach 9y agoThis whole potential problem is one of the reasons the particular example isn't that great. It's overhyped in Java-land where some people treat naked strings like lepers but in places like Clojure-land you'd get a lot of people saying "Just use a keyword" for both places. The keyword's name is its value. It's immutable. An IDE should be able to find all uses of it, or there's always 'ag'. If you really want that coupling behavior, change the interface of the thing you're calling to require conformance to a schema. The simplest way in Java is probably just by taking an enum instead of a string, but there are richer ways (especially outside of Java) worth looking into if you're needing a string value eventually in the implementation since you'll run into the other Java-ism of making a complicated enum class instead of just using enums like keywords with their string value being their actual name value.
- wereHamster 9y agoIf you have one, yes. But I meant when you change the definition of that binding (the right-hand side of the equal sign).
- jsight 9y agoThat is definitely good advice. I've seen some developers get overly obsessed with deduplicating code, and sometimes it really does just overcomplicate things. For example, it is easy to end up making things more brittle by moving a somewhat generic method with very specific pre-conditions and post-conditions into a generic utility library. It is even worse if those pre and post conditions were documented in the original location, but not necessarily copied over to the new shared module.
- sbov 9y agoThe mess of duplicated code is much harder to untangle than de-consolidating code. Especially because subtle differences WILL leak into duplicated code and you won't know if it's on purpose or not. If a dev can't be bothered to figure out where different pieces of code use a variable and whether the change is appropriate for them, do you think they will have the foresight to search for the duplicated code when they need to change it everywhere? Any decent dev should do both, and the former is a MUCH easier process than the latter. Edit: despite the above, I don't completely disagree with your statement.
- dllthomas 9y agoIf they would need to search for the duplicated code and change it in more than one place, there's something shared to factor out. If the two pieces of code are actually only accidentally the same, changing them independently should be the right thing.