I Wrote Redundant Code with Apology Comment

Kasey Speakman - Feb 4 '20 - - Dev Community

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.

. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
Terabox Video Player