loading...

I Wrote Redundant Code with Apology Comment

kspeakman profile image Kasey Speakman ・2 min read

Today I wrote redundant code with apology comment, and it was the right thing to do.

Here is a piece of code that I wrote today. It is a pretty obvious "DRY violation". To the extent that I had to write a comment explaining it. So that hopefully no one would refactor it out later.

type Condition =
    | Org of OrgType
    | ...

// raison d'être: ...
let org x = Org x
let ...

In F#, with the code above, these two things are equivalent.

org Ops = Org Ops // true

Even worse, the one on the left incurs the overhead of an extra function call (if the compiler doesn't optimize that away). There is no good engineering reason for having the extra helper functions. But there is a compelling human reason.

Readability

These types are used to specify access control. They are designed to be used in a sortof code-as-configuration file. Let's compare what each looks like. First without the helper functions.

match command with
| ApproveAudit { AuditId = auditId; FileIds = fileIds } ->
    [
        Org Admin
        Roles [ SuperUser; Standard ]
        Access AdminToAudit auditId
        for fileId in fileIds do
            Access UserFile fileId
    ]

Now let's see what it looks like using the helpers.

match command with
| ApproveAudit { AuditId = auditId; FileIds = fileIds } ->
    [
        org Admin
        roles [ SuperUser; Standard ]
        access AdminToAudit auditId
        for fileId in fileIds do
            access UserFile fileId
    ]

In the latter example, the casing and color differences really help improve the readability of this particular code. You can quickly distinguish the conditions from their values.

Since the subject matter here is access control, I want this to be as easy as possible to get right! And the latter code's readability is better toward that end.

In truth, I made this discovery by accident. I expected I would need to write certain things as functions, so I started that way. When I later realized they would fit cleanly into data types, I started to convert everything over and realized it was less legible.


This example illustrates something that comes up regularly. Trite rules like DRY or "no apology comments, rewrite the code instead" are helpful guidelines, but by themselves they cannot lead you to good solutions for every problem. And taken as absolutes they turn into weapons of civil war against fellow devs.

Follow best practices, but don't let best practices keep you from better solutions.

Posted on by:

kspeakman profile

Kasey Speakman

@kspeakman

collector of ideas. no one of consequence.

Discussion

markdown guide
 

Sometimes, depending on how it's built, the DRY principle is almost impossible to follow without needing to do a rework. Which'll change the estimation ratio dramatically.

Therefore I totally agree with your post, even though the lack of SOLID Principle.

Also, I advise and strongly recommend that every developer should always use English in code documentation.

 

Also, I advise and strongly recommend that every developer should always use English in code documentation.

The irony here is pretty delicious. The comment actually is in English as I don't speak French. raison d'être is a writing device. I have only ever seen it in English articles (since I can't read French). It is just one of those phrases that has transcended its origins. And while I would balk at a dev writing an entire comment in French, I have no problem with common recognizable phrases like this.