When I wrote the first iteration of the Zeebe Node client library in the first months of 2019, I didn’t have any experience in using it. How could I - it didn’t exist.
One early design decision has haunted me for the last year, causing me sleepless nights as I toss and turn, tormented by my guilty conscience.
ZBClient.createWorker() method - to create a new job worker process - takes as one of its parameters an
id, used to uniquely identify a specific instance of a worker in events exported from the broker.
Events viewed in the exporter records contain information like “Worker THX-1138 activated 3 jobs”.
It turns out, after using the library extensively myself, and observing others use it, that this parameter is not widely used (yet). In fact, the Zeebe NestJS integration, (written by Dan Shapir at Pay-K) which integrates the Zeebe Node client with the excellent NestJS framework, auto-assigns ids to the workers. The ability to assign a specific
id for the worker is not exposed to the user.
That’s not to say that it will not be used in the future. In larger systems with more worker processes than the ones currently in use, the id may well be derived from an environment variable, to allow misbehaving worker instances (including out-of-date ones) to be identified and dealt with.
But for now, requiring a worker id to create a worker is clunky.
The source of my problem here is that I violated a principle explained in Bob Martin’s “Clean Code: A Handbook of Agile Software Craftsmanship":
The ideal number of arguments for a function is zero (niladic). Next comes one (monadic), followed closely by two (dyadic). Three arguments (triadic) should be avoided where possible. More than three (polyadic) requires very special justification—and then shouldn’t be used anyway.
I initially had the
createWorker method take four arguments. That was a bad idea.
I later extended it to take a fifth parameter, when I added the ability to attach an
onConnectionError handler. Hey, “if you find yourself going through hell, keep going” - right?
Here is the method signature, with the Generics removed to make it clearer:
The last two arguments are optional, but the worker id is the first parameter. Somehow, when I first wrote it, I thought it was necessary and important.
The solution to this at design time is to use the Parameter Object pattern. That way a function / method takes a single argument (next best thing to none), and it can be extended to add new parameters, and modified to make parameters optional, with no impact on existing code.
I knew the clean coding principle of one parameter. I knew about using parameter objects (see this StackOverflow code refactor where I explain more the impact of avoiding them).
And I did this anyway.
I reasoned at the time:
I’m just banging out some prototype code here to test this out. I can always come back and refactor it later.
“There is no such thing as good coding, only good refactoring”. (That’s my adaptation of a quote by author Robert Graves).
You will always have to refactor as the domain you are modelling becomes clearer, and you materialise and coalesce concerns in your code.
At the same time, this is not one of those “now I can see…” concerns.
I willfully violated a coding principle where I had full knowledge at the time.
Always write the best code possible at the time you write it - while knowing full well that you will have to refactor it later. You are introducing enough technical debt with your keyboard with every line of clean code that you write. You can’t afford to add this as well.
Most of the scratch code you write will go nowhere, and you won’t experience the full impact of sloppy coding practices. It’s easy to write and throw away thousands of lines of code, and go: “What’s the big deal?”
However, the code that does survive will cost you. You should write every line of code as if you have to maintain it for ten years, as a discipline.
Aside #1: I told someone on StackOverflow the other day: “You should always make sure the code runs and is formatted. That will solve 35% of your problems as a professional developer”. Poorly formatted code in questions reveals poorly structured thinking. If you just take on structuring your code properly, the structure of the domain problems become soluble in many cases.
Aside #2: Also, (and I had forgotten this - obviously) when I joined Red Hat in 2004, I wrote code patches for a version of RHEL that was going EOL, after its three-year support lifetime. Well, that day arrived, and some big financial institutions paid millions of dollars to extend its life to seven years. And when that day arrived, paid more money to extend it to 12 years. When I left Red Hat, after 10 years, the patches I wrote in my first year were still in production and being supported. There is no way you can forsee that, so just write all code as if you have to maintain it for 10 years, as a discipline.
You can make trailing parameters optional in TypeScript, but required parameters cannot follow optional ones. So there was no way to make
At the same time, in writing documentation for the library, it became apparent that examples showing the worker id and a task type were complicated for introducing new users to the library.
Everything in that statement is unknown to a new user. Having to explain each element, and say “Ignore this one” leads to the obvious question:
If it should be ignored, why is it even visible?
I also found myself having to supply information to the method every time I used it that was just thrown away.
With over 6k weekly downloads of the library from NPM, and multiple downstream packages, I can’t just change the signature in a breaking way. That breaks it for a lot of people.
The problems of success.
Making a required parameter “optional”
The first thing I did, for my own sanity, is make that first parameter -
id - nullable (assigning a UUID if
null is passed in). So the signature changed like this:
Now you can write (and examples look like) this:
Marginally better. It reduces cognitive load when encountering the library for the first time, and for using it. But my original sin against clean code principles is still there for the world to see.
In statically-compiled languages such as Java and C#, you can provide multiple method implementations with the same name but different parameter signatures. On compilation, the compiler resolves the one you mean from a combination of the method name and the parameter signature, and links the call to the correct implementation.
With TypeScript, you can provide different parameter signatures for the same method, but because all type information is erased at run-time, you have to handle the various signatures in a single implementation.
Redemption from my original sin was going to cost. The internal complexity of the implementation would go up having to deal with this design error in a fully backwards-compatible way.
I held off on it; but finally bit the bullet and did it in the work for the upcoming 0.23.0-alpha.1 release, providing an alternate method signature - one that no longer requires the worker id as an argument.
Why didn’t I just go all the way and move to a single parameter object? Because I’m an idiot, maybe. I wanted it to be a gradual change to avoid having to document and test two radically different signatures. Maybe that’s a mistake that will pile more technical debt in. I’ve designed it to reduce that, by encapsulating the new concern that I’ve added (run-time decoding of the signature).
My plan is to add the third method overload to a single parameter object in a future release, if it makes sense. The jury is still out on that. If it needs extension, that will definitely be part of that work.
This is not released yet, so there is still time for me to deliberate and go all in on the object parameter.
Update: I did it. It has a single parameter signature now, as well.
Creating a method overload in TypeScript
Here is what violating the single parameter principle will cost you, if you have to maintain backwards-compatibility:
To go from this in the README:
While still supporting existing code that uses the previous signature, requires the method to go from this:
You have to provide both signatures as overload signatures, then define an implementation signature that is a union of all overloads. And this is with the Generic Parameterization stripped out to reduce the noise.
But wait - there’s more!
Now you have to decode the call at run-time by examining the actual parameters that you received, and determining which signature the caller used, and which parameter is which.
That is why the implementation signature (not shown to consumers) contains parameter names like
To reduce the internal complexity of the method (that’s about creating a worker, not decoding parameters!), and also to reduce the work required to support an eventual single object parameter, I moved the parameter decoding to a unit testable pure function.
The entire impact on the internal complexity of the method is this:
OK, I’ve isolated the implementation of the method from the fall-out of my momentary madness in creating that original signature.
Now I have to implement the run-time decoder:
That’s a lot of internal complexity required to go from:
to this, with backwards-compatibility:
Going to the third signature, with a single object parameter, will be a small amount of additional code: a new signature overload, a line or two in the decoder, then changes to documentation, and a test or two (because I don’t touch the implementation, just the run-time decoder).
And to think:
All that code could have been avoided by using an object parameter for a method that takes more than one parameter.