5
\$\begingroup\$

Correct User Input - x86 Linux Assembly was closed for being "off topic", but looking at what's on topic, I noticed the likely reason was vaguely stated:

To the best of my knowledge, does the code work?

Is it work as intended, or run without errors? Surely questions about performance and security would also not work as intended (be fast and be secure). Though after reading some of the duplicates, I think that line should be modified to read this:

Does the code produce correct output? If not, your question might belong on Stack Overflow.

\$\endgroup\$
  • \$\begingroup\$ We already had a case, where code produced correct output, but in a way, that was obviously not as intended. The same reason applies there \$\endgroup\$ – Vogel612 Oct 23 '14 at 11:24
  • 2
    \$\begingroup\$ @Vogel612 I'd call the wrong amount of padding incorrect. \$\endgroup\$ – Cees Timmerman Oct 23 '14 at 12:17
8
\$\begingroup\$

Deleted my previous answer, it does not actually address your question....

I noticed the likely reason was vaguely stated:

To the best of my knowledge, does the code work?

Is it work as intended, or run without errors?

Neither. It is somewhat intentionally loose. Defining exactly what 'working code' is will never be successful. What's important here is the 'best of my knowledge'. If you believe the code works, then you also believe the code is ready for review.

If you say: "it does not do everything it needs to do" then you cannot possibly believe it works. If you say: "it fails in condition 'X'" then you cannot possibly believe it works. If the code cannot compile, it cannot work.

Performance and security are interesting because they are not absolute concepts. They are relative. Is it "fast enough", is it "secure enough". As a result, you can believe it is secure enough.

Note, the questoin you reference is from years ago, and before the n-topic help page was edited, but, that question has:

The problem I am having is that there are enormous spaces in the output because I do not know how long the string was that the user entered

It is clear that the asker knows that the code does not produce the desired output, so it is clear the asker knows the code is not ready to be signed off as "looks good". As a result, it is clearly off-topic. The close reason would now say: "Questions containing broken code or asking for advice about code not yet written are off-topic"

Surely questions about performance and security would also not work as intended (be fast and be secure).

No, security and performance are relative concepts, and things can be performant enough, and secure enough. Even slow code can be fast enough, and just because something takes a long time, does not mean it can be made faster.

Though after reading some of the duplicates, i think that line should be modified to read this:

Does the code produce correct output? If not, your question might belong on Stack Overflow.

No. That line will lose meaning. This is not about the code producing wrong output, this is about the code being ready for review. The code is ready when a reasonable programmer believes the code works for its intended purpose, and that he will reasonably expect to get a "Looks Good" stamp of approval on the code.

Ideally, every code review should be a candidate for a "looks good" approval, and every question should be asked with the expectation that it will get one. If the asker knows it will never get a "looks good", then it is not working to the best of their knowledge.

\$\endgroup\$
  • \$\begingroup\$ "If you believe the code works, then you also believe the code is ready for review." Unless you wish to clean up old comments or add comments first. And a review could also point out why it does not work; even the OP can. How would i quickly ensure that my code can't be faster if users complain about it being slow? Is "Can this code get any faster?" off topic? "Reasonable", "intended", and "looks good" are vague; think of style and clarity: Is regex okay? \$\endgroup\$ – Cees Timmerman Oct 23 '14 at 13:38
  • \$\begingroup\$ Also, this is about code producing wrong output. The code looks fine and the output does its job of telling the user what color is favorite, but there's too much padding in the output. The code works and is thus not broken, and askers often don't know better code, e.g. switch or dict vs a bunch of if else blocks. \$\endgroup\$ – Cees Timmerman Oct 24 '14 at 7:39
  • \$\begingroup\$ @CeesTimmerman The code does not work as it is supposed to though. The OP wants to change the output of the code, which makes it off-topic. Changing the output of the code is only one of many possible reasons for why code can be off-topic for "not working" though. \$\endgroup\$ – Simon Forsberg Oct 24 '14 at 11:49
  • \$\begingroup\$ @SimonAndréForsberg IMO, the code looks good, the program runs, and the users gets all necessary data. Cosmetic considerations can well be an external factor. Therefore i believe my proposed sentence is the best to avoid confusion. \$\endgroup\$ – Cees Timmerman Oct 24 '14 at 12:15
1
\$\begingroup\$

Code works when it does that which was intended in full and nothing more, not containing errors for the original expected input domain, with the exclusion of issues caused by external factors such as memory limitations.

See https://codereview.meta.stackexchange.com/a/2536/49350 for a full explanation of this statement.

\$\endgroup\$
  • \$\begingroup\$ I don't think Joe Newbie will know what that sentence means, or care to read much more than that when he just wants a few friendly tips about his code. \$\endgroup\$ – Cees Timmerman Oct 23 '14 at 12:24
  • \$\begingroup\$ That's the full explanation behind whether code works or not. \$\endgroup\$ – Pimgd Oct 23 '14 at 12:30
  • \$\begingroup\$ "such as" is vague. The "full explanation" doesn't address speed and security, for instance. \$\endgroup\$ – Cees Timmerman Oct 23 '14 at 12:34
  • \$\begingroup\$ Those are external factors. \$\endgroup\$ – Pimgd Oct 23 '14 at 12:39
  • \$\begingroup\$ I understand the sentence now, but think my proposed one is simpler and sufficient. \$\endgroup\$ – Cees Timmerman Oct 23 '14 at 12:46
1
\$\begingroup\$

Current wording:

To the best of my knowledge, does the code work?

Your suggestion:

Does the code produce correct output? If not, your question might belong on Stack Overflow.

What we perhaps really would like to say:

Does your code have the features you want it to have and produce the result you want it to?

If you want to fix or change what your code does, your question does not belong here.

In the question you are referring to, it says: (emphasized by me)

The problem I am having is that there are enormous spaces in the output because I do not know how long the string was that the user entered, only the length that I declared the buffer to be.

This is an indication that the code does not work as it is supposed to.

As my usual comment says whenever I encounter a code-not-working-as-it-should question:

I'm afraid this question does not match what this site is about. Code Review is about improving the cleanliness of existing, working code. Code Review is not the site to ask for help in fixing or changing what your code does. Once the code does what you want, we would love to help you do the same thing in a cleaner way!

Can our current wording be improved? Probably yes, there is always room for improvement.

I don't however think that your suggestion is optimal for this. In fact, I'm not so sure that my "What we perhaps really would like to say" alternative is the best either.

\$\endgroup\$
  • \$\begingroup\$ When i'm stumped on code style, there are often still features that i would like to add, but keep on hold while i wait for style guidance, so your suggestion could cause confusion. Technically, a different algorithm changes what the code does, hence my reason for using "correct output". \$\endgroup\$ – Cees Timmerman Oct 24 '14 at 12:17
  • \$\begingroup\$ Saying this site is about code cleanliness is great, but different algorithms can both look equally clean...so performance questions don't belong here? \$\endgroup\$ – Cees Timmerman Oct 24 '14 at 12:23
  • \$\begingroup\$ @CeesTimmerman Code cleanliness is not the only thing this site is about. Performance questions do belong here. Perhaps the best would be to say "in a better way". A different algorithm doesn't change what the code does, it changes how it does it (at least in my opinion). \$\endgroup\$ – Simon Forsberg Oct 24 '14 at 12:29
  • \$\begingroup\$ At the functional level you're right, except when the new algorithm can handle more input or speed/space is an allowed requirement. \$\endgroup\$ – Cees Timmerman Oct 24 '14 at 17:02

You must log in to answer this question.

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