Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

good code may not look like you expect

- duplicate code isn't always bad

Sometimes* the same code copy/pasted or repeated is much easier to understand than pulling it all into a subroutine. It seems logical when writing code, but kills understanding when reading code. Predictable is better than compact a lot of times.

- code should fail early

Sometimes* "being robust", correcting errors or continuing after an error is the wrong thing to do. When something unexpected happens, failing, and failing quickly might be important.

- explicit is better than implicit

Sometimes* it is much better hardcode a list of things instead of constructing the list from whatever is there.

concrete example would be a makefile that compiles .c vs a makefile that compiles one.c two.c three.c. Hardcode the list. Make it hard fail if something is missing or something is added. Yeah, it is more work, but the madness it prevents is worthwhile.

there are probably thousands of these things. All are opinion, like camelcase sucks and indent should be 4, but not with tab characters.

* not always



Strongly agree with your second point, too many times I had to figure out issues caused by code trying to handle errors but actually hiding them and making it so much harder to debug. The typical antipattern: catching SomeErrorType just to log "error of that type occurred", thus hiding the associated error message and the stacktrace. Even worse: trying to catch every error types.

Don't catch errors unless doing something actually useful with them. Even then, it's often a good idea to re-raise.


I also believe people throw way too many exceptions. You should be able to be explicit about the outcome of an error case, and not just throw exceptions in a panic.

So alongside swallowed exceptions is throwing exceptions in scenarios that were entirely recoverable, and I think in both scenarios devs just don't understand their program enough and are choosing to panic, or ignore the problem.

I think people often throw exceptions rather than write the code that handles the situation more gracefully. If you knew it could happen, then is it really an exceptional circumstance?


I think it's impossible to come up with a good rule of thumb for error handling. Sometimes catch-and-log is what you want. Sometimes catch-and-reraise is what you want. Sometimes let-it-fail is what you want. I really cannot come up with a good argument that one of these is the better default. Languages should force us to consider which failure mode we want for every function that can possibly result in error.

The most egregious thing to me is the fact that most languages let any function throw any error it wants without requiring that be documented or part of the type system or anything.


100% agree. “Error” is as big and ambiguous as “success”. What should a successful function return? True / false? An object? A URI? Nothing?

There is no rule of thumb because the scenarios are too varied and context matters.


I try to make sure that the failure happens as close to the cause as possible. For instance, validate input when it's supplied rather than when it's used.


I’d like to add a corollary to this

- Sometimes what’s best for the system isn’t what’s best for your job

Fail fast is a good ethic but all to often I’ve encountered applications that are only to happy to limp along for the sake of optics. If that’s the prevailing sentiment where you work I’d recommend against violating it off your own bat.


> Sometimes* the same code copy/pasted or repeated is much easier to understand than pulling it all into a subroutine.

The Write Everything Twice principle is a much needed sanity check for absurd DRY practices. There are too many naive people arguing with a straight face that writing a base class and two specialization classes saves you the work of writing two classes.


That's one situation where it makes sense.

I had another recently where I need to implement an integration for a specific protocol. It's got a standard but most providers are not completely compliant and it's not mature enough to be exact. An expert consultant asked us to implement a pristine version and then subclass, add rules to change fields, behaviour etc. etc.

I suggested that it would make more sense to just copy/paste the implementation and make tweaks with comments that would accommodate the other providers. Worked like a charm and `diff`ing the files more or less gave us what's different. Got us from 0 to 60% without any problems.


I have started to gravitate toward ”write everything 100 times” or at least ten.

I’m thoroughly convinced DRY is the most ill-applied of all programming dogmas. At every place of work I have been I have run into bugs introduced by changes in one of the cases someone decided to ”dry up” several times. I have never seen bugs being caused by code not being DRY, and I cannot believe someone theoretically in the future maybe forgetting to include a bug fix in some case is worth the hassle.

In very few cases you are solving an actually generic problem. These are usually low hanging fruit already solved in a standard library or are available as an open source package in your eco system of choice. Your business logic is not general.


Minor nitpick that the point is not to save on the upfront effort, but to reduce maintenance burden. If you need to make a change that touches multiple endpoints, then it's usually easier and safer if you only have to make that change in a single place.


> Minor nitpick that the point is not to save on the upfront effort, but to reduce maintenance burden.

There are multiple benefits beyond that. Perhaps the most important one is preventing premature abstractions, which means not only avoiding all the work required to roll them out but also avoiding the work maintaining all the technical debt it creates.


Sure, but not every abstraction is premature; with experience and care, it's possible to walk the line between too much and too little, and premature abstractions can be easier to undo than overdue abstractions (you can always do the copy-pasting after the fact, but if you have two classes that should have common code but have evolved in slightly different directions due to lack of oversight, it can be very difficult to unpick that cleanly)


> Sure, but not every abstraction is premature;

How can you tell if what you're task to do is to duplicate a class?

The wisdom in WET is that it prevents the accidental complexity created by premature abstractions while not preventing the introduction of abstractions later. In the process, development work is also simpler and lower in risk profile because duplicating code only causes the introduction of an independent code path while preventing regressions. Naive interpretations of DRY are renowned to needlessly cause problems due to those mistakes.


> The wisdom in WET is that it prevents the accidental complexity created by premature abstractions

Yes, but...

> while not preventing the introduction of abstractions later.

...not necessarily.

A facetious example to illustrate the point:

I am required to implement DogWidgets and CatWidgets and I have two choices - separate the Widgetness into some kind of base class, or copy-paste.

I take the latter approach. The code is initially easier to read and maintain due, as you say, to reduced cognitive load. Fast forward two years; I'm no longer on the project. Now Joe Junior Developer comes along and over a period of several months, is asked to make a series of changes only to CatWidgets; some of these pertain to Widgetness, but Joe is not experienced or familiar enough with the code to understand that, and anyway has not been told to change DogWidget, so we eventually end up with two divergent implementations of Widgetness. Then somewhat later, Jolene Developer joins the project and is asked to fix an intermittent bug in CatWidget. The bug turns out to be partly due to the original Widget implementation, and partly due to Joe's changes - now Jolene not only has to fix CatWidget, but also examine DogWidget carefully to determine if a) the bug affects DogWidget too, and how, and b) whether or not to fix it, and how (which may be different from the CatWidget fix).

Somewhat later still, the business expands into the BirdWidget market. Rolling CatWidget's and DogWidget's Widgetness back together into a base class is now a very difficult refactoring job that is determined to take twice as long as just copy-pasting DogWidget and making Bird changes; doing the right thing by the code doesn't make economic sense. Now we have three problems.

If I chose instead to implement Widget, and derive DogWidget and CatWidget, then yes, I am imposing some additional burden on future developers to learn how the code is structured. However, now every time a change is made to CatWidget, if the design is good then it should be fairly obvious whether it's a change to Widgetness, or a change to Catness. Bug fixes and improvements to Widget now automatically filter down to both (and all future) Widgets; if Widgets need a colour, then even if we only want to give it to CatWidgets at the moment, we can stub it out or provide a default value for DogWidget, and that thought process also forces us to examine our design instead of ignoring DogWidget altogether, which helps to keep the design current and relevant. Designs should evolve over time.

If one day we get sick of this code structure and wish we had taken the other route, we can quickly copy-paste and completely fork CatWidget with basically no risk of regression, and minimal effort. BirdWidget is obviously trivial to implement. New Widget safety legislation can be implemented once cleanly, instead of three times in three slightly different ways.

> Naive interpretations of DRY are renowned to needlessly cause problems due to those mistakes.

Naive anything is problematic. DRY and WET are tools to be deployed when it is appropriate, not rules to be followed dogmatically; as I said there's a line to walk. As to your initial question:

> How can you tell if what you're task to do is to duplicate a class?

As far as I know there is again no hard and fast rule - it's a heuristic that we develop over the course of our careers: "is this likely to change in the future, and in what ways?" With experience, one can learn to foresee that the business might branch out into the BirdWidget market, and with care, develop code that supports that possible future without being too tied up in abstraction right now. Of course one can go too far, although as I outlined above, I personally feel that 'a bit too far' is overall better than 'not quite far enough', at least in the long run. I've personally spent countless hours trying to untangle legacy WET code compared with much less time necessary to understand DRY code.


> (...) so we eventually end up with two divergent implementations of Widgetness.

That means they were never the same implementation to begin with.

> Then somewhat later, Jolene Developer joins the project and is asked to fix an intermittent bug in CatWidget. The bug turns out to be partly due to the original Widget implementation, and partly due to Joe's changes (...)

Irrelevant. It means you have two components that are buggy. Create two big tickets and tackle them whenever the team has capacity.

How many months or years have passed since your hypothetical initial refactoring?

That's the problem with mindless fundamentalisme involving DRY. Throwing complexity around and tightly coupling components that in practice have no relationship other than superficial similarities are mistakes that only create technical debt in the long run.

> As far as I know there is again no hard and fast rule - it's a heuristic that we develop over the course of our careers: "is this likely to change in the future, and in what ways?"

That's the mistake you're making: fooling yourself into believing that mindlessly throwing complexity around prevents issues and future proofs implementations. It doesn't. You are only increasing the burden to maintain a project without bringing in any tangible positive tradeoff. YAGNI is a battle-tested guideline, which you're violating. Even in your example the introduction of a new widget meant bugs affecting it were either only affecting it, thus lower in risk profile, or already in the original component, which means low risk as no one stopped them. Why are you choosing to make your life harder?


OK, well, all I will add is that I've been where you are on this now, and I've also been an architecture astronaut, and now I'm somewhere in the middle. This is where I've landed based on long experience. I know which code I prefer to maintain, and it's not the big ball of mud that, in my experience, inevitably results from insufficiently abstracted designs. I'm absolutely not making my life harder; I'm making life much easier for my future self, and this is borne out by actual experience on projects I've worked on.

However as I said, there's no hard and fast rules, so, you do you.


The Rule of Three: “When you’ve written the same code for the third time, that’s when you should stop and refactor it into an abstraction.”

I think it's from Martin Fowler's book Refactoring. (It's been a while).

The gist of it is that with one or two cases you might create the wrong abstraction. When you have three, you can abstract away the right bits without creating a monstrosity of optional parameters and booleans.


> The Rule of Three: “When you’ve written the same code for the third time, that’s when you should stop and refactor it into an abstraction.”

Small but very important change: "[...] that's when you should stop and consider refactor it into an abstraction". Sometimes it's needed, sometimes it's not, dogmatic following of that rule lacks the required nuance for real-life programming.


“If it ain’t broke don’t fix it”

I don’t believe it’s wise to add another iteration to working code just for the sake of aesthetics.

There will come a point where you will have to write a fourth implementation and at that point you really should be considering abstraction and reuse of your three earlier known good implementations.


There’s also an argument for abstraction instead of having three different people write three different implementations simultaneously. It lets you stub it out to unblock multiple teams, and centralize learning/iteration. Still imperfect and not always the solution, but worth considering in a move-fast environment.


I like to think of it as that; you should have principles, such as trying to write DRY code, but any principle taken too far will end up being a bad thing. Don't be an extremist. Practice a sensible balance in everything you do.

This goes beyond programming as well, I think it goes for most things in life.


In every trade or art you start as an apprentice. That is the time when you learn the basics, the rules, the best-practices. When you have mastered the state of the art, you are a master. You know when to apply which rule and tactic to create masterful artifacts. The next step is to learn when you should break the rules and general wisdom. That is where true wisdom starts.


100%, on all of these. In basically every job where I worked with more than 5 people, I have regretted too many default arguments in functions that were trying to be overly dry, and code that tried too hard to not crash and carry on.


Regarding your first point, I feel it's important to understand Liskov's substitution principle, which is basically, the code might look the same now, but it could diverge in the future if the contextual semantics around that code are different. Effectively that heuristic has made me a little more hesitant to do a bunch of consolidation that could end up increasing complexity by a lot if you need a bunch of special cases.


Also sometimes copy paste is faster than trying to abstract a code block into something generalised


I think I had discussed every one of these items with my junior developer this week alone. Are you spying on me? Are we the same person?




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: