Should be focus on solving OPs problem or more general analysis

Question

I couldn't come up with a better title so bear with me.

---

Preface (please read):

The private beta is slowly coming to an end so i'd like to have discuss a little about that topic since onces public beta starts the amount of questions hopefully will increase greatly and i want to make sure I'm "on the same boat as you are" dear fellow programmers.

Now i feel this preface is necessary: I seem to sometimes come of like a jerk what might be a combination of "English is not my first language" and that I'm generally not communicating in a very positive manner it seems. I don't mean anything of the following personal or offending in any way and since I'm always worrying about that stuff i wanted to make that as clear as possible !

Main Point:

Currently the highest rated answer on the site is @LRE answer

break this down into several methods - it's very long, meaning it's not easy to read

to that question. Don't get me wrong: It's a perfectly valid answer that focus on the main problem with the posted code even so it doesn't focus on the specif code at hand at all.

While in this example: is_numeric_array() is missing the highest rated question is a rather simple "here is your code" answers to a question that (for me!) literally has "you are doing it wrong" all over it.

(This is getting pretty opinionated now and since i'm not sure how this is handled on meta.* i'm going to assume you will close this if it's not fitting or not answerable)

It answered OPs question, i upvoted it, but i'm worried (for the lack of a better word) that this is "not enough" for a "code review". At least for me one of the first questions every time is "why do you need this" and given my experience with the language at hand code like this points to bigger problems around it. (The need for code like this might be a design flaw)

To ask a question:

Do we want to tell people about things they don't ask for or is that considered impolite / frowned upon / bad behavior ? Since this is beta and a small community (for now !) i didn't want to rely on downvotes but as for your opinion.

Answer 1

I believe telling about things people don't ask for is a huge part of a good code review. Of course, it should be both polite and helpful.

For code that is obviously doing things wrong, small corrections with explanations are OK, but to me a code review is more about pointing that it's sub-optimal (preferably stating why) than fixing the code. Sure, off-by-ones or changing a call to some built-in or a loose cast can and should be fixed inline. Unnecessary quadratic behavior in a long-ish piece of code should be pointed out, not fixed for the OP.

Probably we'll need to learn to avoid up-voting answers that don't consist of a review, as "here's your code" is effective for helping the stray OP, but off-topic in Code Review.

Migrating questions that boil down to "here's my code, it's wrong, write it right for me" to SO might be a bit painful, but would be worth it for keeping this site high quality.

Answer 2

Do we want to tell people about things they don't ask for or is that considered impolite / frowned upon / bad behavior?

If you tell someone something they don't already know, then you helped increase their knowledge in something. That can't hurt, right? Plus...isn't that the idea of Code Review? To help people improve their code? Why would it matter if you address something they didn't ask about; especially if it improves their code.

I think a good answer on Code Review should be a mixture of "text", references, and code - like idealmachine's answer here. He gave specific code examples and addressed things I didn't ask about - which lead to my code being improved. If he had just posted the code, it would've helped, but not as much as the explanations and references that went with it.

Answer 3

While I agree that there needs to be a review, in order to be useful I think it also needs to demonstrate how to fix it. Preferably demonstrating your suggestions either through code or links that are relevant to the OP.