What is the appropriate MINIMUM length of a Code Review question?

Question

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:

Highlight input if empty

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 user 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 Stack Overflow. 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 user 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 user 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.

Answer 1

Should questions like this be closed as off-topic?

No, questions like that are reviewable...

I find that some of the shortest code samples contain large amounts of wrong. For this particular example I found that:

• There is no point in creating an anonymous function here, do function highlight_if_no_val()
• In fact, use lowerCamelCase; highlight_if_no_val -> highlightIfNoVal
• In fact, would highlightEmptyValues not be a better name ? _{Your comment says: /* highlight input, if it is empty */}
• Consider also renaming your CSS class to match your intent
• Cache ($this), especially if you are going to run this potentially on a number of elements
• It is custom to not put a newline between } and );

Thus there is enough to review, and the question is not off-topic.

Answer 2

The length of the code is actually not important.

A short question can contain reviewable code.

int main()
{
for(int i=0; i<=100000000000; i++); // <----- note that the loop doesn't do anything 
return 0;
}

We know what the author is trying to accomplish with this code. It accomplishes the intended task poorly, and we can say exactly why. That constitutes a good question.

Answer 3

There is a necessary minimum, but it is not strictly related to length.

Rather, the minimum related to whether there is enough context to know whether or not the code is good. For a longer code sample, the related bits give enough context for each other.

As a rough rule of thumb, code should have at least three function calls or control flow statements. A single call is almost never sufficient to know what it's for, and I can't think of any two-call cases either.

The code in this question satisfies that minimum.