29
\$\begingroup\$

As stated in the Help Center, one of the criteria for a question to be on topic is:

  • Is it actual code from a project rather than pseudo-code or hypothetical code?

    Details matter! In order to give good advice, we need to see real, concrete code, and understand the context in which the code is used. Generic code (such as code containing placeholders like foo, MyClass, or doSomething()) leaves too much to the imagination.

What is the rationale for that rule?

\$\endgroup\$
1

4 Answers 4

28
\$\begingroup\$

Hypothetical code makes poor Code Review questions, which produce poor answers.

For example, here is a code excerpt that made a poor Code Review question:

bool f(const string& key, const string& value)
{
    // some codes for input check
    exec_cmd("cmd %s %s", key.c_str(), value.c_str());
    // some codes for making return value
}

bool f(const string& key, unsigned long value)
{
    // some codes for input check
    exec_cmd("cmd %s %lu", key.c_str(), value);
    // some codes for making return value
}

Why is this question problematic? It has been stripped of all context, making it hard to give concrete advice. Some of the warning signs are:

  • Generic function name f(). The question is based on a true story — in other words, it's fictitious, hypothetical code.
  • Probable generic string "cmd".
  • Placeholder comments (with insufficient detail to mentally reconstruct the code that goes there).
  • The function should return a bool, but we don't know what that bool represents.
  • We can speculate what exec_cmd() does, and by extension, what f() does, but we're not really sure.

Compare what happens when the author fills in enough details to resolve three of the five deficiencies:

bool set(const string& key, const string& value)
{
    // some codes for input check
    redisCommand("SET %s %s", key.c_str(), value.c_str());
    // some codes for making return value
}

bool set(const string& key, unsigned long value)
{
    // some codes for input check
    redisCommand("SET %s %lu", key.c_str(), value);
    // some codes for making return value
}

Now, we can see exactly what the code is supposed to do, and can give better advice. For example, previously we could only speculate that there was a security hole in the code. Now that we know that it is trying to set a value in a redis database, the mitigation advice would be completely different.

We don't normally allow code in questions to be edited like that, since it invalidates existing answers. However, in this case, the original question was so poor that allowing the revision was a less bad option. Ideally, the original question should have been closed as "not real code" instead of being reviewed, to avoid such a mess.

\$\endgroup\$
17
\$\begingroup\$

Hypothetical code is difficult or impossible to review because it is a moving, shape-shifting target. For example, @elibyy asked:

https://codereview.stackexchange.com/q/60562/9357

for example i'm deleting file if it exist

if(file_exists('path/to/file')){
    unlink('path/to/file');
}

An answer was given:

You could just call

@unlink('path/to/file');

… to which the original author responded:

however my question is in the abstract perspective, the delete file is example

When code is hypothetical, it can mean whatever the author wants it to mean, and we can't read the author's mind. When unlink() doesn't actually mean unlink(), "reviewing" code is a waste of time for everybody.

\$\endgroup\$
8
\$\begingroup\$

In Keeping track of byte count in a binary protocol handler, we had an example of a question that originally contained stub code that did not accomplish very much. After being put on hold, the author added more code. It then became apparent that there were deeper problems that needed to be addressed.

\$\endgroup\$
-12
\$\begingroup\$

A hammer can be used as a tool and as a weapon . Is that a reason to prohibit the use of a hammer?

Problem is that sometimes original code have too much irrelevant code. So if you add whole code than reader will have problem to read everything, plus you will get comments that are not relevant to your question. I usually need help with general problem, when I implement something but I am quite sure that can be done better. I don't care about better naming, or better some other not relevant thing that will be done later or not, because is not important for me. So hypothetical code is good to keep focus on general point of question, and to get straight answer. Ofcours there is risk with hypothetical code if is not good enough, but that is problem in person that use hypothetical code in wrong way. It is not problem in hypothetical code.

\$\endgroup\$
7
  • 10
    \$\begingroup\$ You not liking advice on better naming and such isn't a good reason for allowing hypothetical code. \$\endgroup\$
    – Jamal
    Jul 31, 2015 at 14:42
  • 3
    \$\begingroup\$ Related: Can we ask reviewers to not focus on something? \$\endgroup\$ Jul 31, 2015 at 15:09
  • \$\begingroup\$ @Jamal Not it is not. But hypothetical code is very useful when your orginal code is too complicated. So you can keep focus on point using hypothetical code. I many cases my questions would be much better and cleaner with hypothetical code. \$\endgroup\$ Aug 3, 2015 at 6:06
  • \$\begingroup\$ Then a hypothetical variation can be posted on Stack Overflow or Programmers. \$\endgroup\$
    – Jamal
    Aug 3, 2015 at 6:08
  • \$\begingroup\$ @Jamal: There you will get short answer wih some good advice (or not). But I want code review focused on point. Anwer I get on codereview are are much helpful to me. Problem is that sometimes it does not make sanse to copy-paste my orginal code, its to complicated, and to hard for reading for somone that is not familiar with my project. \$\endgroup\$ Aug 3, 2015 at 6:27
  • 1
    \$\begingroup\$ Then perhaps Code Review doesn't always suite your needs, and that's not a bad thing. Plus, if unfamiliarity with someone else's code was a problem, then this site probably wouldn't even be around today. Learning to understanding someone else's code is the point. \$\endgroup\$
    – Jamal
    Aug 3, 2015 at 6:51
  • 1
    \$\begingroup\$ @merso0027 I do tend to agree with you. If I can identify the specific PATTERN of code that gives problems, then I can eliminate TENS or even HUNDREDS OF LINES of code completely unrelated to the problem, yet still retain a perfectly functional demonstration of the problem, the understanding of which would allow me to correct the actual problem, and convey to others both the pitfall and the correction of an anti-pattern. Alas, apparently such is not the mindset of CR. \$\endgroup\$
    – user125462
    Dec 11, 2016 at 13:31

You must log in to answer this question.

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