12
\$\begingroup\$

What do we do when OP says his solution works, but the contest engine says it doesn't? Should we give it the benefit of the doubt, as "It works to the best of my knowledge.", or close it as broken code?

This is the question that brought me here. OP says it works for the test cases, but is getting the wrong answer when he submits his solution.

\$\endgroup\$
  • 4
    \$\begingroup\$ I have locked the post on the main site. It is my personal opinion that this particular question is asking for the code to be debugged, and not reviewed. Locking it seems appropriate to avoid other people answering and making the process less clear. \$\endgroup\$ – rolfl Aug 13 '14 at 14:14
  • 1
    \$\begingroup\$ @rolfl I think that the question needs to be closed and I think that Heslacher agrees with me on that. Does anyone else agree? \$\endgroup\$ – Malachi Aug 13 '14 at 14:27
  • \$\begingroup\$ I'm agreed @Malachi. Close it. \$\endgroup\$ – RubberDuck Aug 13 '14 at 14:29
  • \$\begingroup\$ @Malachi, agreead \$\endgroup\$ – Heslacher Aug 13 '14 at 14:33
  • \$\begingroup\$ Note This particular question is related to an ongoing code competition. I have locked the main post again, and I expect that other questions posted related to this competition will be locked/closed/deleted as well. \$\endgroup\$ – rolfl Aug 13 '14 at 19:16
9
\$\begingroup\$

If it is a matter of a time limit, then the question is valid. This is because we would be reviewing performance, but if the application is giving the wrong results then it is broken code.

This question looks like it is not giving the right results based on what the coding challenge constitutes as optimal playing.

Your task is to find out whether Alice can win, if both play the game optimally.

So I would say this is a matter of broken code that isn't doing what it is supposed to be doing.

I haven't parsed the code yet. but I think that might be the issue here.

\$\endgroup\$
  • \$\begingroup\$ You are right. After a quick glance the if both play the game optimally. isn't taken into account. \$\endgroup\$ – Heslacher Aug 13 '14 at 13:59
4
\$\begingroup\$

I think the rules should be changed a little to more closely reflect what I think is our real mission.

Code Review is really about refactoring--improving code without changing its (externally visible) behavior.

If code is so badly broken that its behavior clearly needs to change before it stands any chance of working at all, then refactoring is basically pointless.

In real life, however, it's fairly routine to decide that code with known bugs is worth refactoring to at least some degree. If that clarifies/simplifies the code to the point that some of the bugs are easy to fix, doing so is perfectly acceptable--but (for example) those bugs may be sufficiently minor in real use that making the code more readable and maintainable is a higher priority than fixing bugs.

That leaves something of a judgement call, but (IMO) generally a simpler one than we're faced with today. In particular, rather than being a judgement call about whether the code is bug-free to the best of the OP's knowledge, it becomes a judgement call about the intent of the question being asked. If the question is really about fixing the code, then it belongs on StackOverflow. If it's really about refactoring the code, then it can be a perfectly decent fit for CR, even if it has outstanding bugs.

\$\endgroup\$
  • 2
    \$\begingroup\$ Your answer appears like something that is more general than this particular question. Are you saying that this specific question should be on topic, or are you suggesting that there should be a more general discussion about where the off/on topic line is? \$\endgroup\$ – rolfl Aug 13 '14 at 14:23
  • \$\begingroup\$ Given that this meta question is tagged [specific-question], this answer is a little off-topic, but it could make for an interesting meta discussion. \$\endgroup\$ – nhgrif Aug 13 '14 at 23:40
  • \$\begingroup\$ Related: meta.codereview.stackexchange.com/questions/1859/… \$\endgroup\$ – Simon Forsberg Aug 14 '14 at 20:08
  • \$\begingroup\$ I disagree with the claim in the second paragraph — code review is not just about refactoring code without changing its behaviour. Very often the behaviour is wrong too. \$\endgroup\$ – Gareth Rees Oct 6 '14 at 20:17

You must log in to answer this question.

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