Specifically, how do you check "Correctness of algorithms"? What is the criteria to pass that? "Well, I think it's right", of course you do, otherwise you wouldn't have committed it. Formal proof? Unit tests?
Items phrased like this are ripe to become a "looks fine, check" style process-theatre that doesn't actually add anything.
Credit where credit it due, "Are controls laid out in a way that makes sense given visual scan order?" is a bloody good one.
Checklists can be very helpful to make sure you remember to consider everything in the right order. I have a phone screen checklist that includes "introduce yourself" because sometimes I forget and that's embarrassing. At a previous job, we had a "stakeholders that might be affected" checklist used in feature planning (we had gotten into a bad habit of building features that affected the performance profile of the app, causing ops to get paged on the weekend because they hand't been told to expect different behavior, also: removing an arcane feature that we had wrongly assumes nobody used).
One thing to remember when comparing with flight or medical checklists: These people deal in repeating almost the same thing as consistently as possible. Software engineering is rarely like that.
It's not really meant as a yes/no; it's meant to prompt the code reviewer (not you!) into checking the algorithms for insanity. Easy to forget that when you're rushed and hassled and wanting your lunch.
Also, aren't the majority of flight checklists for abnormal situations? And specifically to make sure that things are handled in a sane and sensible fashion when everything is going off klaxon-wise and/or on fire?
So, what you're describing is actually a good yes/no question: "Is the best algorithm for the problem used?" (or, better: "Has functionality that is available in a library been implemented by hand?", but that's not your point :) )
No, flights use checklists for even completely routine thing (especially for routine things, because that's where sloppiness will manifest itself first). For an example, if you forget to set flaps correctly before you hit the throttle for take off, you're going to have a bad time, so that's on the checklist. (Of course, these things are largely automated now, so modern aircrafts will yell at you if you don't. I think Airbuses will even override the throttle)
Rather: "Is the algorithm good enough?"
I think checklist level thinking for code reviews is bad though. It ends up being a pretty soul destroying experience when someone tells you that you need to validate arguments to all methods as not null (for the 100 you wrote), or that you should take the data in your test cases and put it in a text file. Or that you've re-used the same string in several places and should make it constant. Robots can tell me that, and I can get their feedback while I'm developing or choose to ignore it. People are invaluable for helping you step back and see how to re-organize your code, introduce new abstractions, re-interpret requirements, and see actual bugs that no QA person or tool would ever discover (likely affecting some unlucky user who would hit an extremely rare bug that couldn't be reproduced easily). Those types of reviews are invaluable. Put down the checklist and automate it instead.
One of the most comprehensive tools for doing automated quality reviews of code is Sonar ( http://www.sonarsource.org/ ). It supports pretty much all programming languages (thought not all for free).
For example, for PHP code sonar runs a combination of PHP_CodeSniffer, PHPMD, phpDepend and phpunit and integrates the reporting of all those tools.
My first experience with Java was at a job in high school... and my only reference was a book that weighed half as much as I did. Needless to say it left me with a very, very poor impression of the language, which I'm only recently learning is very much wrong.
I definitely prefer writing Python, but the more I actually learn about Java, the worse I feed for being such a senseless detractor for so many years.
the overall appearance of the checklist does make it seem like it would be problematic, but i think the author makes a good point of showing that they act as a template for the reviewer rather than as strict set of guidelines.
in a situation where "we're all consenting adults here," having a set of things to evaluate features or code style makes it easy for anyone to know what to expect when checking in a change or when reviewing one for the first time. it may not work for you, but it certainly adds consistency when they're used non-dogmatically.
Forcing a checklist upon all reviewers isn't what the article is all about, but rather it's about one developer's way of organizing his own personal process for doing code review so he doesn't forget something.
If I have a huge change in front of me, and I know that I tend to look for certain things, I could certainly try to look for all those in one go. That's error-prone, though. Might forget something, or get bogged down in syntactical stuff and miss something important. However, with a little personal checklist in front of me, I can go "Okay, I'm done looking at syntax. Now let's make sure that there's no off-by-ones." Leads to better organization of thoughts.
To be honest, the only reason we are using C++ is because it's mandated by the project, and quite frankly, it's what we're good at (we'd probably be working another C++ project if this one wasn't in C++). And yes, we know C++ is a kludge; quite frankly, I wouldn't trust a C++ "expert" who didn't question design decisions of C++.
http://www.projectcheck.org/checklists.html
including a "checklist for checklists" (!)
http://www.projectcheck.org/checklist-for-checklists.html
Great article on the use of checklists in hospitals, by Atul Gawande:
http://www.newyorker.com/reporting/2007/12/10/071210fa_fact_...
(Offtopically, "Complications" is also worth a read.)
[1] how do you abbreviate The New Yorker?
And so we'd get into a situation where we'd have to decide whether a particular item was even applicable or not, which somewhat defeats the point of checklists, since you're supposed to be able to decide immediately whether an item is satisfied or not.
"Well now it is, and there's a unit test for it, and you won't be able to commit code that breaks that unit test. Problem solved."
What I do find useful is to have a good variety of skills on the review team. Different people tend to check for different things. Some are just naturally good at spotting, say, concurrency problems, others have a pet peeve with names that aren't clear, etc. But someone who doesn't have an eye for concurrency problems won't magically start noticing them in a pile of code because there's a box saying "check for race conditions" somewhere on the list.
