In Defense of Repetition In Code
I recently came across a lot of people123 saying that this code from the game Balatro is awful:

I wouldn't say it's ideal, but I also don't think it's awful. There is plenty to defend about the approach here, and not nearly enough of that defense in the various comment sections discussing it for my tastes, so I'm going to go ahead and defend it myself here. I'm by no means an expert in Lua, but probably neither are most people attacking this, so I figure we're on even footing.
Simplicio4
That code is horrible! How can you even defend this? Look, the numbers 2 through 10 all do the exact same thing! Even if you must leave the special case face cards, why wouldn't you at least condense those numerical ones down, and save yourself some typing?
Let's try it!
if self.base.value >= '2' and self.base.value <= '10' then self.base.nominal = self.base.value; self.base.id = self.base.value;
elseif self.base.value == 'Jack' then ...
Is this better? I don't think so. Before I only had to think about the single value and the single equality comparison, but now I need to think in two modes: one for the numerical cards, and a completely different one for the face cards. In unifying the number cards, we disunified the approach.
Simplicio
Okay, well I don't think that's a real thing to care about. Besides that, it IS better!
Well, no, because it no longer works! Those are strings, not numbers! And as the Lua documentation5 points out
Lua compares strings in alphabetical order, which follows the locale set for Lua. ... When comparing values with different types, you must be careful: ...
2<15is obviously true, but"2"<"15"is false (alphabetical order!).
Nothing will satisfy this check, since '2' > '10', so nothing will be both higher than '2' and less than '10'. I didn't know that about Lua before writing this post, so already, we've introduced something that requires language-specific knowledge, which makes it less readable for novices.
And no, we can't switch to 2 and 10 directly, because the documentation also notes that Lua does not have type coercion, and attempting it causes a crash.
Simplicio
Obviously I didn't mean compare the string values. You should convert them to numbers before testing inequality. And keeping everything on one line is cheating to just make this look more complicated, any competent programmer would indent it properly.
Alright, sure! Let's try that
if tonumber(self.base.value) >= 2 and tonumber(self.base.value) <= 10 then
self.base.nominal = tonumber(self.base.value)
self.base.id = tonumber(self.base.value)
elseif self.base.value == 'Jack' then ...
We've fixed it, right?
Simplicio
Yes! We're getting there now. However, the whole point of this exercise is to stick to the Don't Repeat Yourself principle, and those multiple tonumber() calls aren't very DRY
Sure, let's go with that.
numValue = tonumber(self.base.value)
if numValue >= 2 and numValue <= 10 then
self.base.nominal = numValue
self.base.id = numValue
elseif self.base.value == 'Jack' then ...
This looks correct, but it will crash. The value is sometimes going to be letters instead of numbers, remember, which will produce nil6 and crash upon comparison with a number5. Sure, whatever, let's just check for nil first.
numValue = tonumber(self.base.value)
if numValue ~= nil and numValue >= 2 and numValue <= 10 then
self.base.nominal = numValue
self.base.id = numValue
elseif self.base.value == 'Jack' then ...
What do we think Simplicio, is this better?
Simplicio
Yes! We've stuck to DRY and minimized the amount of repetitive typing we had to do. You can't honestly tell me that this is worse than the original
I can.
We have lost information that the original block told us immediately:
- There are only integers
- Every integer in the range is a possibility
- The number cards do generally less "stuff" than the face cards
We have even introduced extra required knowledge to this. The behavior of tonumber() on non-numerical strings is integral to the logic, along with the presence of short-circuiting behavior on conditionals to know that if the first condition fails (i.e. numValue is nil) then it won't go ahead and crash on the comparisons afterwards. We've also broken the clarity provided by the number literals in the original block; now we have to keep in mind that numValue and self.base.value are similar data, being transformations of each other, which is a detail the original never required.
I also can't overlook that this doesn't do the exact same thing as the original block. Decimal values will now have an effect in this version, when they wouldn't in the original. It's unlikely (playing cards are pretty fixed, after all) but that's all the more reason to not introduce the possibility of a subtle future bug here to save a whole 5 lines of code.
Simplicio
Oh come on, this is clearly nitpicking. This is much more natural to read for any programmer. It's obvious just from looking at it what tonumber() does, your intuition will probably land on the correct interpretation.
How hard was it to read before? You surely understood everything it was doing near-instantly, right?
I sincerely believe that the thing that made the original block so upsetting to you and your daffy pals was that it was too readable. Understanding the code so quickly was somehow offensive. You knew there were language features that could make this shorter, and when faced with this scenario yourself, you would use them. But I think for something with a dozen cases like this, and a few of them with special cases, writing them all out makes everything so instantly clear that it is worth the cringe of violating DRY. It conveys to the reader the real intent: that you want to do something specific for each card, not a range of cards.
And this talk about "natural" and "intuition" is true of experienced programmers, but the most common skill level in programming is novice, and leaning on these developed skills as proof of code quality will make you harder to work with, and more frustrated with your coworkers, in all but the most siloed, expert-only projects.
Simplicio
Yes, okay, fine. It was more quickly understandable in the original than in my preferred version. But readability doesn't really matter, because it's faster this way! You're only doing a maximum of 3 comparisons instead of the 9 we were doing before. You should never sacrifice speed for readability. Speed is the universal metric by which all code should be judged.
Your preferred version sacrifices speed for readability too! Why not cut out the inequality comparisons altogether?
numValue = tonumber(self.base.value)
if numValue ~= nil then
self.base.nominal = numValue
self.base.id = numValue
elseif self.base.value == 'Jack' then ...
Since the only numbers will always be in the range we specified, it's superfluous. You really only need to know whether tonumber() produced a nil or not to know whether to take the branch. The tradeoff for this performance, though, is losing more information. The range of inputs has transformed from explicit to implicit. The only reason you know what the acceptable range of inputs will be is because you saw the first block with that information as clear as day! A second developer coming to this block will not have that information, and she will need to look elsewhere to find it and carry that knowledge in her head back to this block.
Performance is not the universal metric, time is. Performance measures how fast code executes at runtime, and readability measures how fast a programmer can understand the code well enough to make meaningful changes to it. Getting a piece of code to execute in 0.1ms instead of 0.2ms can often be a worse optimization than making it take 1000ms to fully understand instead of 30000ms7. The time it takes to type the code is even less relevent, since you're doing that once ever and we have clipboards and tab-completion for a reason.
You might think you can disregard readability on solo development projects (as Balatro is), but even in those cases, the other programmer coming to your work is your future self, who ought to regard your present self as a fool anyway if you're growing as you should, so don't bother trying to avoid humiliating yourself in front of them.
Simplicio
This is stupid. This small piece of code does not deserve this level of scrutiny and the miniscule ease of reading can't really determine whether it's good or bad code.
Oho, just a moment ago you were ready to use six inequality comparisons as a pillar of quality, but now marginal gains in readability don't matter?
Well, I'd agree that it's not a big problem, but reading speed adds up much more quickly than speed gains in my experience. I also wouldn't necessarily say that the original code is good; there are probably ways to refactor it that make it clearer without sacrificing readability, like having a table that maps string keys to associated values, but I honestly don't know enough about Lua to suggest them and they require knowledge well outside the scope of the offending screenshot anyway. I was simply taken aback by the apparently universal disdain for repetitive code in any situation and circumstance and a demand to chop it down. DRY is not a moral imperative. It can easily cause problems when applied too aggressively8. I feel the many detractors would do well to learn it the easy way now before they run into its faults in the wild and learn it the hard way.
https://news.ycombinator.com/item?id=43196349↩
https://news.ycombinator.com/item?id=43196422↩
https://www.reddit.com/r/programminghorror/comments/1cb6rca/source_code_from_balatro/↩
Taken respectfully from A Mathematician's Lament, referencing the Socratic dialogues in Dialogue Concerning the Two Chief World Systems, and also the socratic dialogue of The Eoten Manifesto which is much sillier.↩
https://www.lua.org/pil/3.2.html↩
https://www.lua.org/manual/5.1/manual.html#pdf-tonumber↩
Okay, okay, yes, there are exceptions. Software bloat is a problem and being too liberal with this principle can only make it worse. If your code is running in a serverless environment where your execution time directly impacts your cost, sure, performance might weigh more strongly than readability. If there is code that is a bottleneck to the critical path and runs repeatedly in small time frames, small time gains can make a big impact that is noticable to users, which is really the downside of software bloat, so performance can be more highly valued than readability there. Lower readability in these cases might even be preferred: if you're making changes to code that is so critical to the program, you really should sit down and have to study it before making changes. But if the code runs once a month, even saving multiple full seconds off execution does not really help anyone at all.↩
Simplicio is going to fall for this one every single time. It is easier to merge similar code later than it is to untangle unified code into special cases.↩