| ▲ | ryeats 9 days ago |
| This is a trap junior devs fall into DRY isn't free it can be premature optimization since in order to avoid copying code you often add both an abstraction AND couple components together that are logically separate. The issues are at some point they may have slightly different requirements and if done repeatedly you can get to a point that you have all these small layers of abstraction that are cross cutting concerns and making changes have a bigger blast radius than you can intuit easily. |
|
| ▲ | zahlman 9 days ago | parent | next [-] |
| If you notice that two parts of the code look similar, but have a good reason not to merge or refactor, that deserves a signpost comment. If you're copying and pasting something, there probably isn't a good reason for that. (The best common reason I can think of is "the language / framework demands so much boilerplate to reuse this little bit of code that it's a net loss" — which is still a bad feeling.) If you rewrite something without noticing that you're doing so, something has definitely gone wrong. If a client's requirements change to the point where you can't accommodate them in the nicely refactored function (or to the point where doing so would create an abomination) — then you can make the separate, similar looking version. |
| |
| ▲ | chipsrafferty 9 days ago | parent | next [-] | | I don't think it's as cut and dry as that. In my team we require 100% test coverage. Every file requires an accompanying test file, and every test file is set up with a bunch of mocks. Sure, we could take the Foo, Bar, and Baz tables that share 80-90% of common logic and have them inherit from a common, shared, abstract component. We've discussed it in the past. Maybe it's the better solution, maybe not. But it would mean that instead of maintaining 3 component files and 3 test file, which are very similar, and when we need to change something it is often a copy-paste job, instead we'd have to maintain 2 additional files for the shared component, and when that has to change, it would require more work as we then have to add more to the other 3 files. Such setups can often cause a cascade of tests that need updated and PRs with dozens of files changed. Also, there are many parts of our project where things could be done much better if we were making them from scratch. But, 6 years of changing requirements and new features and this is what we have - and at this point, I'm not sure that having a shared component would actually make things easier unless we rewrite a huge amount of the codebase, for which there is no business reason. | | |
| ▲ | matijsvzuijlen 9 days ago | parent [-] | | I can understand requiring 100% test coverage, but it seems to me that requiring a test file for every file is preventing your team from doing useful refactoring. What made your team decide on that rule? Could your team decide to drop it since it hinders improving the design of your code? | | |
| ▲ | chipsrafferty 6 days ago | parent [-] | | Honestly, I've never questioned it. The upside seems that yes, it could make refactoring easier. Downside, though, is not knowing where certain tests live. The command+shift+T shortcut to open the test file corresponding to a production code file is very useful to me, too. Thanks, I'll have a think about it |
|
| |
| ▲ | smallnamespace 9 days ago | parent | prev [-] | | > If you're copying and pasting something, there probably isn't a good reason for that. I would embrace copying and pasting for functionality that I want to be identical in two places right now, but I’m not sure ought to be identical in the future. | | |
| ▲ | fauigerzigerk 9 days ago | parent [-] | | I agree completely. DRY shouldn't be a compression algorithm. If two countries happen to calculate some tax in the same way at a particular time, I'm still going to keep those functions separate, because the rules are made by two different parliaments idependently of each other. Referring to the same function would simply be an incorrect abstraction. It would suggest that one tax calculation should change whenever the other changes. If, on the other hand, both countries were referring to a common international standard then I would use a shared function to mirror the reference/dependency that they decided to put into their respective laws. |
|
|
|
| ▲ | rkomorn 9 days ago | parent | prev | next [-] |
| The reverse of that is people introducing bugs because code that wasn't DRY enough was only changed in some of the places that needed to be changed instead of all the places. To me, it's the things that are specifically intended to behave the same should be kept DRY. |
| |
| ▲ | pkolaczk 9 days ago | parent | next [-] | | An obvious example of that is defining named constants and referring them by name instead of repeating the same value in N places. This is also DRY and good kind of DRY. | | |
| ▲ | ryeats 9 days ago | parent [-] | | This is actually a particular pet pieve of mine because I worked with the Camel framework which has a lot of boilerplate in strings but if you start using constants for the common parts you now have an unreadable mess of constants concatenated together that buys you nothing. |
| |
| ▲ | sroerick 9 days ago | parent | prev [-] | | This is the correct take - if you're getting this type of bug, it's now past time for DRY |
|
|
| ▲ | fenomas 9 days ago | parent | prev | next [-] |
| All my younger colleagues have heard my catchphrase: Copy-paste is free; abstractions are expensive. |
| |
|
| ▲ | kragen 9 days ago | parent | prev | next [-] |
| DRY isn't an optimization of any kind, so it can't be a premature optimization. "Premature optimization" is a specific failure mode of programmers, not just a meaningless term you can use to attack anything you don't like. "Optimization" is refactoring to reduce the use of resources (which are specifically cycles and bytes) and it's "premature" when you don't yet know that you're doing it where it matters. Otherwise I mostly agree. |
|
| ▲ | seadan83 9 days ago | parent | prev [-] |
| Indeed a trap. I'd say DRY is all about not duplicating logical components. Just because two pieces of code look similar, does not mean they need to be combined. As an analogy, when writing a book, it's the difference of not repeating the opening plot of the story multiple times vs replacing every instance of the with a new symbol. |
