16
\$\begingroup\$

Related:

I feel that it is currently too easy to copy some code that you have written and put it up for review here on Code Review. My opinion is that if you want a review of your code, then you should also be able to provide an explanation about the code.

Horrible examples of question with far from sufficient descriptions:

(Yes, three of these also have horrible titles, but that's an entirely different subject)

OK examples of questions with some description provided, but it could include some more details/overview/comments of the code: (Depending on how you look at them, you could argue for both sufficient and not sufficient description)

Good examples of where sufficient details are given to understand what the code is doing:

\$\endgroup\$
2
  • \$\begingroup\$ Out of interest, I disagree with your assertion that Integer Partition is a good example... it really was not, and took some guessing/investigation to even understand the code's intention... I would rate it as 'bad', not even 'OK'. \$\endgroup\$
    – rolfl
    Dec 8, 2013 at 22:48
  • \$\begingroup\$ @rolfl It definitely wasn't at first, but was then edited and the description was "The aim is to enumerate all possible sets of whole numbers whose sum is a given integer" which made it much more understandable IMO. But of course it's possible to have different opinions about this. I'll actually put it in "OK" instead since there are far better examples. \$\endgroup\$ Dec 8, 2013 at 22:51

4 Answers 4

7
\$\begingroup\$

I'd like to generalize the complaint: code to be reviewed should include sufficient context.

In particular, I've noticed that many questions are frequently bad at providing sufficient context for a code review. I've added the following admonition to its tag wiki:

If you request a code review for an SQL query,

  • Provide some context about what your query is trying to accomplish.
    • Does it as an offline batch job on a reporting database? Or does it run on your primary database, and the user expects an immediate response?
    • Does the code execute the query in one shot, or does it execute the query many times in a loop?
    • Are there any tables where the number of rows could be a concern?
  • Include sufficient information about your database schema (table definitions)
  • If query performance is a concern,
    • Ensure that indexes are in place, and mention them when describing the schema.
    • Include the output of EXPLAIN SELECT.

… and called attention to it in the tag wiki excerpt:

Read the tag wiki's guidelines for requesting SQL reviews: 1) Provide context, 2) Include the schema, 3) If asking about performance, include indexes and the output of EXPLAIN SELECT.

If you see SQL questions that lack sufficient context information for a review, please add a comment referring the poster to the tag wiki. In addition, it may be appropriate to vote to close as "Unclear what you are asking" or downvote the question for low quality.

\$\endgroup\$
2
  • \$\begingroup\$ I completely agree, and noticed the changes to that wiki \$\endgroup\$
    – Malachi Mod
    Dec 9, 2013 at 20:05
  • \$\begingroup\$ I believe this is applicable for all questions not only SQL. Downvoting the question and adding comments seems to be the best way to handle this. In some extreme cases, questions might be closed as well but I think the down-vote approach is more preferable. \$\endgroup\$ Jan 18, 2014 at 18:50
4
\$\begingroup\$

If you can't figure out what the OP is asking after reading the question and asking in the comments, then you should vote to close based on being unclear as to what the OP is asking.

It sounds like there is another post somewhere that elaborates on this.

\$\endgroup\$
3
\$\begingroup\$

New close reason proposals was already addressed by @codesparkle:

This [close reason] limit applies to all sites with specific exceptions (not sure if there are any except for Stack Overflow) as explained in this MSO answer. I'm not convinced that there's an urgent enough need for this to warrant asking for an exception though. The wording of the proposed new close reason also leaves a lot to be desired.

Basically, we can only have three close reasons at the moment. The ones we have (non-working code, code behind a link, and code not yet written) are just fine. I do agree entirely with @James about this being what comments are for. If comments don't help reveal more info from the OP, then the question can be closed with one of the existing close reasons, namely unclear what you're asking.

\$\endgroup\$
2
\$\begingroup\$

Proposal: This deserves a close reason.

(This is just a first attempt at formulating one, suggestions for formulating are welcome)

To help the reviewers understand your code, questions should contain a description about your code in addition to the code itself. The longer the code, the more description should be provided.

\$\endgroup\$
4
  • 6
    \$\begingroup\$ I think thats what comments were designed for? asking for clarification. (we also have a close reaon: unclear what you're asking.) \$\endgroup\$ Dec 8, 2013 at 22:54
  • \$\begingroup\$ @JamesKhoury "what they are asking" is often clear: "I want a review of my code". Yes, comments could be used, but I felt that we needed something "stronger". This is what I could come up with at the moment. I'd love to see other alternatives as well :) \$\endgroup\$ Dec 8, 2013 at 23:02
  • 2
    \$\begingroup\$ As codesparkle mentioned before, we can only have three close reasons at the moment. The ones we have are just fine. I do agree entirely with James. If comments don't help reveal more info from the OP, then the question can be closed with one of the existing close reasons. \$\endgroup\$
    – Jamal
    Dec 9, 2013 at 0:25
  • \$\begingroup\$ @Jamal Can one of you add an answer for that, so that instead of seeing how it should not be handled, people can see how it should be handled? \$\endgroup\$ Dec 9, 2013 at 12:28

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .