Reviewing NChord: A codebase that makes me twitch

time to read 2 min | 283 words

I was referred to NChord by one of the comments, and it looked very interesting on the surface, so I downloaded the code and took a peek.

It is not typically my style to criticize code openly, especially after a very brief glance, but I thought that it  might be a good idea to point some of the things that made me decide that the codebase isn't worth the time to pursue in details.

Single Responsibility Principle doesn't apply to the file level


What you see here are no less than 11(!) files that are all parts of the same class. That is.. bad, from a whole lot of reasons, not least of which is that you have to jump through a lot of files to understand what is going on, and that ChordInstance is at least a demi-god, rapidly moving upward.

Then we have things like this:


Now, this is a matter of taste more than anything else, but it strongly hints that there is lack of threading experience going on here. Indeed, a brief look at the code shows numerous potential race conditions.

Code like this really make me twitch:


All of that trend toward making me doubt the maturity and stability of the codebase.

In short, after a very brief scan, I find numerous issues that I really didn't like in the code base. And I am not willing to put up the effort to try to make headway into this with so many early warning signs already in place.