# Reviewing non-working code when author thinks it's working?

I came across a pretty simple question where the author explicitly states:

The function returns the correct output, and all known edge cases have been accounted for.

I took this at face value and started writing up a quick review. Examining the code further, it turned out to be broken. I went ahead and finished answering, since my remarks were a bit long for a comment.

The answer got an upvote and an hour later, a drive-by downvote. Not sure what the downvote was for, but I wonder if it's a convention here to downvote answers to non-working code, and whether the answer should be removed?

If we take a look at https://codereview.stackexchange.com/help/on-topic we see

To the best of my knowledge, does the code work as intended? if the OP answered this for him/her-self with yes, then the question should be on-topic (if the other questions on the on-topic section could be answered with yes as well).

but I wonder if it's a convention here to downvote answers to non-working code

If a question is clearly off-topic then yes, thats the default behaviour, but such a downvote should always come together with an comment like "Please refuse to answer clearly off-topic questions."

See this meta:https://codereview.meta.stackexchange.com/a/7054/29371

• Other relevant metas are here and here. – Mast Jul 25 '19 at 6:08
• Ahh, "to the best of my knowledge" is key here, thank you. – user11536834 Jul 25 '19 at 6:13

Unfortunately, your question is based on a faulty premise. The code to be reviewed consists of a makeBst() function with one test case. That test case produces incorrect output:

class BinaryTreeNode {
constructor(value) {
this.value = value;
this.left = null;
this.right = null;
}
}

function makeBst(arr){
if(!arr || arr.length <= 1){
return arr;
}
let top = arr[Math.floor(arr.length / 2)];
let node = new BinaryTreeNode(top);
let rightArr = arr.splice(Math.floor(arr.length /2), arr.length);
let leftArr = arr.splice(0, Math.floor(arr.length / 2));
node.right = makeBst(rightArr);
node.left = makeBst(leftArr);
return node;
}

const arr = [1, 2, 3, 4, 5, 6, 7];
console.log(makeBst(arr));

The output:

  4
/ \
1   6
/ \
4   7
\
7


So, applying some principles for evaluating whether the question is off-topic, I would conclude that the author should have been fully aware that the code is broken. I would consider the claim that "The function returns the correct output, and all known edge cases have been accounted for" to be a lie.

It would be unfair to allow the author to get the blatantly broken code reviewed (and possibly get a free bugfix out of the review) just by making a false claim that it works. You can't work around the "to the best of your knowledge, does the code work?" requirement by lying. This question should therefore be closed as off-topic.

I realize that you had invested time to look at the code before realizing that you have been misled. Unfortunately, that happens sometimes. The way to discourage such unfair behaviour is to boycott these questions, not by answering them.

• "the author should have been fully aware that the code is broken" -- I agree with that completely. I wasn't quite ready to conclude that it was a lie, because I figured the user could have simply posted it on SO and asked, "what's wrong with my code?" ...so I gave the benefit of a doubt, thinking he honestly may have not realized it's broken. But I guess that would require a pretty astounding level of ineptness, and after some reflection I suspect the user was simply trolling. +1 for taking the time to point this out. – user11536834 Jul 31 '19 at 8:24
• Not sure what to do with checkmark in light of this answer. This question was really seeking information about the general case, not so much about that specific question, and I feel like the accepted answer does address the general case. But it's definitely worth considering that the question could be intentionally misleading, for whatever reason. – user11536834 Jul 31 '19 at 8:28