13
\$\begingroup\$

There are some questions that are technically off-topic but still feel that the code really needs our help.

For example: https://codereview.stackexchange.com/questions/49347/tic-tac-toe-artificial-intellenge-work-fine-but-here-i-have-to-click-two-times-f

The question asks:

/Tic Tac Toe Artificial Intellenge work fine but here i have to click two times for player chance and also for Computer Chance ? Please Help and thanks in advance..?? How Make computer take its own chance/ SORRY FOR BAD LANGUAGE..:)

And then some code follows, for example:

    else if(tttc.button[6].getText().equals("<html><font face=Jokerman size=15 color=red>O</font></html>") && tttc.button[7].getText().equals("<html><font face=Jokerman size=15 color=red>O</font></html>") && tttc.button[8].getText().equals(""))
        {
        tttc.button[8].setText(TicTacToePlayer.letter);
        tttc.button[8].setEnabled(false);
        }
    else if(tttc.button[1].getText().equals("<html><font face=Jokerman size=15 color=red>O</font></html>") && tttc.button[2].getText().equals("<html><font face=Jokerman size=15 color=red>O</font></html>") && tttc.button[0].getText().equals(""))
        {
        tttc.button[0].setText(TicTacToePlayer.letter);
        tttc.button[0].setEnabled(false);
        }

If it wasn't for the fact that the question is phrased as "Please help me change the behavior of this program", rather than "Please review my code", it would be on-topic. I feel that many such questions contains code that is quite easy for us to review, but still we shouldn't help them with the off-topic request of course.

I see a couple of alternatives about what can be done with these questions:

  • Ask the poster to fix the issue first and then come back and get the code reviewed.
  • Ask the poster to re-phrase the question to ask for a Code Review, ignoring the fact that parts of the code doesn't have the intended behavior.
  • Review the code as it is without changing the behavior of the program.

There is likely other alternatives about these kinds of questions as well.

My point is that: The question contains reviewable code, so should we perhaps consider reviewing it? Or what should we do?

\$\endgroup\$
4
  • 4
    \$\begingroup\$ Closely related to Could incomplete or not-quite-working code be suitable for review? That question asks about slightly broken code; this question asks about working code where the author asks for help with an enhancement. \$\endgroup\$ May 9, 2014 at 16:31
  • \$\begingroup\$ I like the two answers, but I think this is a major cultural shift, and it can have potentially astronomical impacts on this site, with possibilities ranging from hypothetical degradation of overall content quality to... possibly nothing less than site graduation through somehow managing to deal with a phenominal growth rate spike as we become The Best Thing Since Stack Overflow. \$\endgroup\$ May 10, 2014 at 1:10
  • 2
    \$\begingroup\$ @Mat'sMug: That's also why I disagree with this. Even if we try to review the working parts, someone else may end up fixing the broken code. The close reason already mentions updating the post with working code, and I think that's good enough. I'm sure SO is full of reviewable code, but we should stick with working code. \$\endgroup\$
    – Jamal
    May 10, 2014 at 2:33
  • \$\begingroup\$ The question that you picked as an illustration is closer to non-working code than working code. You might have gotten more sympathy had you picked a more worthy example. \$\endgroup\$ May 10, 2014 at 23:23

4 Answers 4

9
\$\begingroup\$

Almost Working code

Code Review is for working code. Let's play a what-if game... (devil's advocate) ... what if it's OK to have a rule that says: "Almost Working Code is on-topic".

That means:

  1. "I tried Poject Euler #3x and it is running really slow, and it gets the wrong prime factor for 654321234567, but here's the code, how can I improve it? It compiles!"
  2. "This code is part of my event bus system, and it normally works, but occasionally when there are many threads running, it does odd things. Can you think of ways to improve the code?"
  3. ......

Now, back to the current situation, no more hypotheticals ....


Purpose of a Code Review

The purpose of a good code review is a plural:

  1. Conformity and maintainability - does the code conform to the standards and expectations of your peers... will other people understand what you are doing?
  2. Performance and efficiency - are there ways to make the code go faster, or do what its doing more efficiently
  3. Are there edge cases that may cause the code to fail
  4. Is the code doing the right thing in the typical/normal case.

These items give a measure of confidence to a piece of code. This 'stamps' the code with an 'approved' blessing: The code will do what it is supposed to do, reasonable situations are covered, and it will do the job well, and will be maintainable.

But, what is most important? The most important item is the last item. Code that does not accomplish the normal task correctly. Without working code, the other items are pointless.


Prioritization

Now, about those questions that are "almost working"? What is the problem with them in Code Review? The problem is both simple, and challenging.

A good code review prioritizes the worst-offending parts of the code, you tackle the biggest problems first. You show where the most significant parts of the code are, and you then work outwards.

When you have code in front of you, the most pressing issue is always: does it work?

If it does not work, then it is useless. The only reasonable course of action for a decent code review is to say: "Well, let's start with the most significant issues, and that is the basic functionality. Once we get it working right, we can tackle the other issues, like indentation, overly-nested loops, etc. Maybe we can fix them all at the same time."

If the code is not working as designed, and the code review does not fix that issue, then having pretty code that does the wrong thing, fast, and efficiently, is not an improvement.

In other words, the most critical issue is: does the code work as designed?


Jurisdiction

The answer is no. The code is Almost Working, and that means the most important output from a code review would be working code.... and, guess what?

That issue is squarely the primary scope for Stack Overlflow.

If the code does not work as designed, then the only logical place where the question is on topic, is Stack Overflow. That will solve the most pressing problem. That will help the asker the most. That is the first step.


Conclusion.

"Almost Working" questions/code does not belong on Code Review, it belongs on Stack Overflow.

Why?

  • they are specifically on-topic there
  • they are the immediate problem, and the remaining code review can happen after
  • it is impossible to measure what "almost working" means.
  • it is safe: A question on SO can be answered by the people on Code Review just as easily
  • if Code Review people want to debug code they could spend time on Stack Overflow. In fact, many people do spend time on both sites.
  • Stack Overflow is populated with people who want to do this. This it the purpose of Stack Overflow, and they want these questions.
  • just because people on Code Review often invest more time in answering questions, and just because we have brilliant people who are willing to share their time, and knowledge, it does not mean that we are the right people/place to do the menial work... ;-)
  • This is not a "no, your code cannot be reviewed" situation.... it is "... your code cannot be reviewed yet!". The asker is welcome to bring their code back when it works.

Yes, I am snobbish. I feel that the caliber of help that people get (and give) on Code Review is exceptional. Code Review is not only for trivial code, but for big and juicy things as well. We are not a first-line support system. We are third line people...

Stack Overflow is struggling with the workload of fixing issues. The truth is that Code Review could add significant help for those people who are struggling to get their code to work. We have very knowledgeable people, and experts in their fields. But, by inviting people to bring their code here when it's not quite working, it will be doing both Code Review, and Stack Overflow a disservice.

If you want to help people to get their code working, then the right choice is to spend more time on Stack Overflow, and help them with their issues (both the meta problems in SO, and the problems that people ask for help with).

When those people have working code, remember to recommend that they post it here!

\$\endgroup\$
3
  • \$\begingroup\$ Excellent analysis, but it answers the linked related question (almost-working code) more than this one (working code with a request for help to enhance something). Or is there no distinction between the two in your opinion? \$\endgroup\$ May 10, 2014 at 16:36
  • 3
    \$\begingroup\$ @200_success if the code is not doing what the asker wants it to do (either missing feature, or broken feature), then the highest-priority is to get it working as designed, and that is primarily on-topic on SO. \$\endgroup\$
    – rolfl
    May 10, 2014 at 17:40
  • \$\begingroup\$ @rolfl But the shortest way (and also the one leading to the best learning effect) may be to review the code. Quite often, I ignored some bugs and cleaned the code and the bugs were gone or became obvious. \$\endgroup\$
    – maaartinus
    Oct 21, 2014 at 13:17
4
\$\begingroup\$

Rather than applying the rules blindly, I think it helps to consider the rationale for our rules. In particular, two of our criteria for a question to be on topic are:

  • Asking for code to be written is off-topic
  • Must contain working code to be on-topic

The main reasons for those rules are:

  • We must have some code to review. Otherwise, it's just not Code Review.
  • Requiring the code to be in working state enforces a quality threshold.

Asking for help with a minor enhancement, within reason, is actually not a problem. In fact, just posting a question is implicitly seeking advice to improve the code. We regularly rewrite programs, sometimes from scratch, in the course of answering. In particular, many questions of the form "Review my code, which exceeded the time limit for a " usually require a completely new algorithm. And we routinely provide such algorithms, or at least hints at fresh approaches.

Conversely, forcing the author to jump through unnecessary hoops by rewording the question to our liking would result in a poor user experience, with no real benefit to Code Review in terms of improving the quality of the questions. Referring the author to another site when the question is mostly on-topic for Code Review would also cause unnecessary frustration.

So, if a question asks for help with an enhancement, the guideline should be, Has the author made a reasonable attempt at solving the problem? If so, I think it would be mutually beneficial to provide the requested help. If not, then just review the code as-is, and add a comment saying that addressing the requested enhancements would be done at the reviewers' discretion, since asking for code to be written is off-topic for Code Review.

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

Well let me phrase it with the words found in the edit privilege help

When should I edit posts?

Any time you feel you can make the post better, and are inclined to do so. Editing is encouraged!

Some common reasons to edit are:

  • to fix grammatical or spelling mistakes to clarify the meaning of a
  • post without changing it to correct minor mistakes or add addendums /
  • updates as the post ages to add related resources or hyperlinks

You should not edit questions that are off-topic (at least not without consulting OP), so that's out.

I personally think the answers here are so good and teach OP so much, that we should not prevent him from receiving valuable feedback on his code. But that is exactly what we would do when closing the question while asking OP to come back. Also most users don't come back, which I find a little sad.

So I'd also rather not close, as that prevents further answers. But for that to happen we need the question rephrased, as we should not allow definitely off-topic questions to remain open. This would confuse new users.

TL;DR;

I would ask OP to rephrase his question, and review the code as-is. As soon as we change the behaviour of the code, it's not a codereview anymore.

This also means we'd need to rethink our definition of working code.

Possibilities in overview:

  • Editing: Out. Would be too invasive.
  • Closing: Out. Not helpful to OP and bad for user-retention.
  • Answering, including the behavior change: Out. That is not a codereview..
  • Answering, no changes to behaviour: The about only thing left ;)
\$\endgroup\$
1
1
\$\begingroup\$

Status Quo.

We're doing great the way we have it. Broken code is off-topic, period.

\$\endgroup\$

You must log in to answer this question.

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