Obviously related to this, but I want to know the opposite. How short is too short? For example this doesn't seem long enough to be appropriate for a code review:
The entire code sample:
/* highlight input, if it is empty */
$(function() {
var highlight_if_no_val=function () {
$(this).toggleClass('highlight-input', !$(this).val());
}
$('#id_index-EditForm-gpnr').keyup(highlight_if_no_val).each(highlight_if_no_val);
}
);
Clearly this guy has a different understanding of what a code review is than I do. This doesn't seem like enough code or nearly enough of a complete picture to warrant a review. Should questions like this be closed as off-topic?
Should testability be mandatory?
The length of the code sample is an oversimplification of only part of the problem. A bigger problem is the lack of context. We don't know what the markup being "queried" looks like, we don't know what else is going on in the script, etc.
You can't easily test this code by passing it some values. That means you can't refactor it and be sure that it still works as before given some test cases.
Without testability, we have to guess at stuff and say things like "do something like this instead" and leave FIXME comments here and there, etc.
Making at least some level of testability mandatory could really improve the overall quality of the code review requests listed here.
What's the solution?
Reviewing (answering) these requests seems like the wrong move to me. It encourages quantity over quality.
Closing the questions as off-topic might be easiest. Another approach could be moving to SO. The justification for that could be that the code sample is short enough and not enough context is given that only one or two things can really be said about it, making it a question suitable for SO.
In the example above, the question is introduced like this:
This snippets works, but I think it can be optimized:
[code]
Is there a simpler solution?
In fact this already sounds like an SO question, and probably has a duplicate on SO, so it can just be moved there, marked as a duplicate, and closed (which sort of seems like a lot of trouble). This guy doesn't really know what question he's asking, but that's OK, others do. When he clicks through to the duplicate he'll understand what his question was, and see the answers.
Where should the mark be set?
In contrast to the previous example, it's like this guy read my mind. Sure, it's a TGP browser. Sure, he's using Comic Sans MS. But I can drop his code in jsfiddle, test it, edit it, and give him a good code review. He tells us what the program does and gives the script, markup, and css all in one code block.
There's plenty of stuff to review, and plenty of code, enough that you don't feel like you have to refactor the whole thing (and at the same time nothing too irrelevant).
If the mark for valid questions that involve multiple technologies (here, as above, js, css, and html) is set closer to this example than the example above, I think a lot of noise can be avoided.
