4 ms·
I work at a place that requires code review and has automated checkers for "code quality". Let me tell you the obvious : the best code I see has these problems,
by waps 12y ago
I work at a place that requires code review and has automated checkers for "code quality". Let me tell you the obvious : the best code I see has these problems, because the best code is the best code because it uses the language to it's maximum effect, which often precludes having perfect "style". The best code is tested in functional ways, which means that it doesn't have "good coverage", nor does it really use unit tests more than a little bit.
And the worst code I see, by people who learned to code a month ago or worse and don't know any algorithms (but feel like they know better than people who've studied algorithms and languages for years) almost without exception perfectly styled. It contains moronic errors like swapping variables around (because these programmers do not know how to use the type system), it contains 5 unit tests for every single little function, because that increases the lines of code metric that is so universally used. And the reason for half the lines of good is "good practices".
Here's how you recognize good code : firstly it is not possible to shorten it without causing a MAJOR disaster in the readability area. Every concern is properly separated out into it's own pieces of code, and aside from the (short) main function there is very little single-purpose code. The typing system is used well. Prices and amounts are NOT the same data type, for example, and cannot be obviously switched. Unit tests exist for core algorithmic pieces only and other than that there are system tests that confront the code with real-world situations while running almost the entire program, ideally under heavy load with half the backend unresponsive, and has a statement that says the test fails if it takes longer than 1/10th of a second. It implicitly follows and beautifully implements a design document that is not written in word, but in a 10 to 15 line comment on top of the file/class.
Note the issues :
1) hardcoding things
2) naming mistakes
Both of the issues you complain about can be fixed mechanically or with absolutely minimal supervision through refactoring. Yet next to all the comments below suggest DOUBLING the manual effort needed to get code into the repository. While automated fixing might be problemating, writing linters that detect these problems is trivial.
Unless of course the problem is that you feel that you need to fix how others program because you "know better", but can't actually write code checkers. In this case, why are you leading them ?
So well in that case I'd advise a slice of humble pie, and a compilers course.
> I review commits now and then and find some of these issues - but due to the nature and timeline of the project cannot do it for each and ever commit of course.
You sound like someone with an MBA. A programmer would recognize this for what is is : a problem screaming to be automated. Code commits can be made dependant on code checkers succeeding - just write the ones you want/need. Can't do that ? You're not a programmer - or at least not a good one - and stop whining about how difficult programming is - learn it first.