12
\$\begingroup\$

This question here: Shortest possible way of printing a specific board

Summary:

  1. The code requirement is to print out a grid of text.
  2. The code does in fact print the grid
  3. The way it does that, is very broken, and it is only by hacking things that it works.
  4. From a code-review perspective, it does not work 'as designed', but only 'by mistake'

In cases like this, the most significant issue is to get the code to do the right thing, not to make it do the right thing well. The end-result (the output grid), is not the only measure that should be used to calculate 'the right thing'. The 'right thing' should also consider whether the algorithm works in a reasonable way....

As a test, I answered the question in a way that is more suited to Stack Overflow, then to Code Review. The answer received a fair amount of positive votes, and no negative votes.

Is this really a good question (for Code Review)? Is it a good answer (for Code Review)?

Bottom line: Should the question be closed as "Off topic, code does not work" even though it produces the right result?

\$\endgroup\$
  • 3
    \$\begingroup\$ To the best of my knowledge, does the code work? seems to apply here. I say it's on-topic, and I'd upvote your answer if I had ammo! \$\endgroup\$ – Mathieu Guindon May 26 '14 at 18:49
  • \$\begingroup\$ "In cases like this, the most significant issue (...)" that part sounds like a self-answer rather than a part of the question. \$\endgroup\$ – Simon Forsberg May 26 '14 at 19:14
10
\$\begingroup\$

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

It is very hard to tell, almost impossible actually, if the OP is aware of the flaw in the code.

As I don't know the skill of the OP, here's what I think happened:

  1. The OP wrote the initial program.
  2. The OP noticed that one of the lines in the output was a bit off and therefore added the if-else code to adjust the output.
  3. The OP noticed that the program was outputting the correct result, but felt that the if-else approach was not a good one and therefore asked a question on a StackExchange site.

Summary: I don't think the OP was aware of the problem. Based on these things, I have to estimate the skill of the OP.

What the OP needs

The OP needs to understand his own code. To determine whether or not the question is on-topic, let's take a look at our magical six questions:

  1. Does my question contain code? (Please include the code in the question, not a link to it) Yes
  2. Did I write that code? Yes
  3. Is it actual code from a project rather than pseudo-code or example code? Yes
  4. Do I want the code to be good code, (i.e. not code-golfing, obfuscation, or similar) Yes
  5. To the best of my knowledge, does the code work? The big question mark, but based on what I wrote above: Yes. It works, even if it's by mistake, it works.
  6. Do I want feedback about any or all facets of the code? Although not explicitly written, probably yes

The big "problem" here is that the OP is lacking a lot of knowledge.

Why your answer is a good answer:

You explain what is going on and how to do it correctly.

What your answer didn't mention that I've seen in other C++ reviews:

  • using namespace std;
  • Indentation

Mentioning these things could possibly have made your answer a better answer from a Code Review point of view. However, based on the OP's knowledge, there are more important things to learn first.

Here or Stack Overflow?

  • Would the question be on-topic on Stack Overflow? Probably. However, a lot of users there would probably suggest that the OP post it to Code Review so it could possibly have ended up here anyway.
  • Is it off-topic on Code Review? No.

Another Stack Exchange site alternative would be Code Understanding (Area 51 proposal). Even though this question isn't about understanding the code, understanding the code is what the OP needed.

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

Reviewers find bugs all the time. That is, after all, one of the benefits of submitting your code for review.

Nevertheless, Code Review is not a debugging service; we require the code to be in working order, to the best of the author's knowledge.

So, the question is, what did the author know, and when did he know it? It's hard to definitely prove that the author knowingly posted non-working code, but there are some heuristics:

  1. Are there any doubts expressed in the question itself?

  2. Does it fail to compile?

  3. Does it fail to run, or to produce the expected output?

  4. If it's not obvious from the heuristics above, then the next question I would ask is, would a person having ordinary skill in the art of programming have recognized it as broken code?

    One way to assess what a PHOSITA would think is to see if other Code Reviewers were able to spot the bug. It is surprising how non-working code sometimes manages to sneak through a whole phalanx of reviewers. For that reason, this question remains open, despite what I personally consider to be very obviously broken code.


So, applying those heuristics to the specific question…

  1. The question contains no explicit doubts about the correctness of the program. In fact, the author is primarily concerned about code length — so much so that it was originally posted to Code Golf.

  2. It compiles.

  3. It produces the expected output.

  4. Base on the tests above, I would assume that the author was blissfully unaware of the brokenness of the code. The question is therefore on-topic for Code Review.

    Hypothetically, if the first three heuristics were inconclusive, then I would say that a PHOSITA would be able to see that it was broken code, with the tip-off being the special case needed for i == 0.

\$\endgroup\$
5
\$\begingroup\$

I feel that you handled it as best you could given the circumstances. Ideally, it would have been migrated to SO for correction, but it wasn't for two reasons:

  • Original PCG mods determined the poster didn't want short code--just better code--and didn't notice it was broken, so they migrated it to CR.

  • You figured out that it was broken code but couldn't migrate it again to SO because the system doesn't allow it. [Is there a feature request for this ability?]

At this point you had a few choices:

  • Migrate it back to PCG and have them migrate it to SO. This would involve some back-and-forth and doesn't really help the poster for a while.

  • Close it as off-topic. Not very helpful either.

  • Point out the flaw and help them fix it.

The last choice seems like the most helpful in this situation. I would add instructions to the poster to fix the code, post a new question to get a proper review, and delete the old question. If they do so, we now have a new happy CR user. If they don't after some time, simply delete it since it's not useful for anyone else as it lacks a review and the OP obviously doesn't care.

\$\endgroup\$
  • \$\begingroup\$ Also, note that the question would have needed to be rephrased for Stack Overflow. \$\endgroup\$ – 200_success May 26 '14 at 19:57
3
\$\begingroup\$

If you talk purely about "is code working", then I presume you do not take a specific CodeReview approach to that problem. Hence "is the code working" should be approached as a black box. You put stuff into the program, it spits correct pieces out. Hence the code is working.

Now we are here at CodeReview to ensure that everything is coded correctly and that the code does not violate any design principles. I think we are at the first part here, checking if everything is coded correctly. If something is coded via two bugs that counter each other, then that is not a correct approach.

However I think this question really is borderline as there doesn't seem to be an effort to try to get it working correctly.

\$\endgroup\$
  • \$\begingroup\$ To make an effort to do it correctly, you'd need to know why and what does not work, which is clearly not the case here... \$\endgroup\$ – Vogel612 May 26 '14 at 19:03

You must log in to answer this question.

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