How many times have you set out to make an application vowing to keep the code clean and sensible till the very end and miserably failed midway? Do you feel the bad code, that’s barely holding everything together, haunt you when you see someone else use it?
My teammates and I have been through a similar journey lately while working on a small project called Code Character, an AI programming challenge where users write C++ code to control troops to battle against enemies!
One can use the interface to write code on an online editor, and submit their C++ code to see a rendering of their game after it’s run on the server. There’s also built-in version control on the web interface. Finally, you can submit your best codes and fight against others on the leaderboard! It is a reasonably sized project from a frontend perspective given the number of features that are needed to be implemented into a single page application. Though there were a large number of challenges we faced when developing, I wanted to put into words the stages we went through in terms of the effort put in to keeping the code clean.
We used React-TypeScript with Redux, Redux-Sagas for async actions and webpack with CSS modules.
Note : This is one of our experiences as a couple of college students working on something out of passion and not any enterprise software :)
Stage 1 — Let’s make the cleanest app possible
This is the stage where we are actually enthused and try to make use of all possible tools and swear to write and maintain clean code.
TypeScript on strict? Check! Linter? Check! Code Formatter? Check! Tests? Check! CI? Check! Strict branch protection rules? Double Check!!
We used TSlint with Airbnb ES6 rules for linting. Prettier for code formatting. Mocha-Enzyme for testing. CircleCI for checking linting, tests and build. We also had a minimum of 2 code reviews for every PR to master when there were only 3 of us working on it. Pedantic? Of course not!
Everything feels perfect.
We overcome our laziness and write that extra test for that extra check even if it feels like it can never ever go wrong.
We review that 10 line PR for 20 mins checking for variable names, best ways to write a function, adding comments to things that need clarity and enforce strict rules on PR formats, commit names, commit changes and descriptions.
Not able to find the TypeScript declaration for that npm package? Well search again. Still didn’t find? Write your own.
Stage 2 — This rule is stupid! We don’t need it.
This is the stage of personal preferences .
TSLint requires me not to pad blocks with newlines? Well, that looks ugly. Let me set this rule to false.
TSLint wants the default import name to be camel-cased package name. This cannot be right in all cases right? React Components have to start uppercase. Let’s disable TSlint for this line.
Okay, this package does not have a TS declaration and has too many things happening? It isn’t my job to declare its interfaces. TS IGNORE!!!
I shouldn’t console.log even in catch**** blocks? F**k off. TSLINT DISABLE!!!
At this point, you see a couple of TSlint disable and ignores here and there in reviews. But still it’s under control, it’s just the linting. Nothing can go wrong. CI log shows a couple of warnings, but it’s fine.
Stage 3 — This doesn’t need to be tested. Right?
This is the stage of assumptions .
This string can never have special characters. This number can never be non-decimal. This number can never become a string. This string can never become an array. This array can never transform into an object.
It’s just stupid to test it. I should be writing tests for more important cases that can actually happen.
It’s all green in CI status checks. It all works together fine and things still seem to be under control. But are they really?
Stage 4 — I trust you. I’m approving the PR. Actually, I’m just lazy.
This is stage of lethargy .
This variable could’ve been named better, but it’s fine.
There is a better way to implement this function, but this works too. Approve. He could’ve added those CSS changes to the commit description, but should be obvious without it. Approve. There isn’t a lot happening in this PR. It won’t affect anything anyways even if it fails. Approve. She definitely knows better than I do. I don’t think I need to even review it. Approve. He should’ve written a more strict test for this check, but this test should fail too if it goes wrong. Approve.
It still looks good. But silently everyone knows it really isn’t.
Stage 5 — Shit. Red cross on CI!
This is the stage of repentance .
This error has nothing to do with the current changes. Why is this breaking now? Shit. This case should’ve never happened. Small fix and a test should be enough.
Another error on the same test? This test has been annoying lately and it isn’t even that important. Let me just remove it.
Build is failing? It passed till now and I didn’t fucking touch the part that throws the error! Ohh Right, my bad. Didn’t know that could ever happen. I’ll just send in a quick PR to fix it.
Wait, it builds on my local, why does it fail on CI? This makes no sense. Something’s wrong with the package’s version I guess. Let me just switch back to a stable old version and find a fix later.
Everyone now knows it isn’t under control at this point and we have accepted our inevitable fate.
Stage 6 — F**k tests. Fuck CI.
This is the stage of acceptance .
Now we are almost at the deadline (*Extended Deadline) and there are a couple of important things that need to be pushed.
Let just the build work. The tests are too strict anyways and it’s affecting productivity. All this code still goes through review right? Surely, things can’t go that wrong. We’ll fix the tests when we have time. Remove CircleCI checks for this repository for now. Let’s ignore TypeScript for a while, it’s just unnecessary effort to find the right types anyways.
CSS has linting checks? Not anymore.
Everyone is just working on hacking it together and getting all the features done now. Code cleanliness is the least of concerns at this point.
Stage 7 — F**k Master.
The app is now on the server and deployed.
Need to make this minor CSS fix. Feels stupid to send a PR to master for this. Let me make a “temporary” branch. Here comes server-fix (or whatever you’d name it)**** branch. This feature needs to go live asap. Push to server-fix ! Need to change a tag href. server-fix !! Made a typo. server-fix !!!
Uh Oh… server-fix now has too many commits. I’ll send PRs later for each feature.
Alternatively, we start force pushing to master after removing branch protection rules.
It’s all over now. Other than code formatter, nothing else is being respected at this point. All features work, but sanity is lost. This code can now only be understood by the ones who’ve written it.
Conclusion
Meeting deadlines and completing features correctly can be a wild ride, especially when working on your own project in a small team, without a concrete set of requirements. You make the rules, and of course, you break the rules too.
After that entire harrowing experience, it’s finally out there to play!
Check out Code Character, an AI programming challenge where you write C++ code to control troops of your own and challenge others!
Happy Coding :)