So, someone shared some code on Github and some classic developer snark rolled in. And then there were some apologies about it. I saw those snarky tweets when they came through, clicked to the Github project, didn’t understand what the issue was, and went back to work.
Now, I’m all for bad projects and bad code being called out. Our industry produces some shitty code, and a general lack of
craftsmanship can kill business, or even people. So bad code needs to be pointed out. The question is, is this bad code? I
think we can all agree that if it is bad code, we should try to point it out in a more constructive way (and I know it’s easier
said than done), but let’s forget the tone and focus on the message. Is
replace a silly project, or implemented in a stupid
No, it’s not.
I think we can evaluate a project based on three criteria:
- should it exist at all?
- does it use the right technology?
- are the lines of code well written?
Should it exist at all?
replace is a command line app, which is something near to my heart. Writing a command-line app to automate
something tedious is what good developers do (even if you can wrangle
mv etc. to do it without scripting
it). The use case of
replace is one I’ve needed many times, and, when I did, I hacked together some pile of shit shell script
to do it one-off and then promptly deleted it because then no one would know my shame in having made such bad code.
So, I’d say that it’s completely reasonable that this exists and wouldn’t be surprised if many people found it useful. I believe
this is why I initially didn’t understand the negative reaction. I saw “some command line that makes search/replace across files
way easier than dealing with
sed and friends” and figured that was useful and good.
Does it use the right technology?
In Avdi Grimm’s take on this he writes:
But I do share the opinion of a number of my colleagues that using a reactor-based framework in a language lacking native fibers, coroutines, continuations, or threads leads to messy code.
I totally agree with Avdi on this, but I don’t think it’s that relevant to the choice of technology here. It turns out that if
have async as an option, but it’s not like this thing pops up a web server and gets lost in callback hell. And, even if it did,
Is the code itself well written?
For a command-line app, I would say it’s pretty good. There’s a proper option parser in place, with useful and complete help text, and all configureability is done via command-line options - as it should be. The main thing this project is lacking is automated tests, but it does include some sample files that can be used to exercise the app’s functionality via manual testing on the command-line (that’s certainly better than many of the command-line scripts I’ve written over the years).
The main code itself, all contained in
replace.js, is fairly straightforward. The main routine is a bit complex, but it’s
not hard to follow and method extraction has clearly been used to keep the main routine readable. I’m sure it could be done more
cleanly, but it’s pretty good as it is.
So, I’d say this code is pretty well written.
I won’t claim to be great at this, but it’s something I spend time on trying to improve, even if it’s at the cost of learning some new language or framework. It’s an ongoing process, and this blog post is part of that process.
I’ll leave you with some actually bad code from a bad project - one of mine: a Java project to parse a YAML-like format and turn it into XML. At the time, I’d never heard of YAML, and the entire thing uses regexes for parsing, along with a giant mess of a “doit” method that’s screaming for a refactor. Feast your eyes on this. It should never have been made, nor should it have been done in Java, and the code is quite a mess.