2
\$\begingroup\$

As requested by one of the mods, this is the first of 3 suggestions/requests made here
I've copy pasted it with some edits, during my lunch break, so if this suggestion doesn't make too much sense on its own, please refer to the original post. I'll edit this post further once I find the time...

I'd like to suggest adding a notice to the help section of code-review, sort of like jslint warns you that using JSLint will hurt your feelings. Where I live, we have this saying that, loosely translated says "Gentle healers make stinking wounds"(Zachte heelmeesters maken stinkende wonden). Same applies to CR: being too careful and too reassuring might decrease the quality of the resulting code... (New) users should be aware of the fact that good code-review can and indeed should be tough

warn new users that CR has to be tough, to be good.

Here's why I think this should be added:

This site is dedicated to code-review, that much is obvious. But what is code-review exectly? More to the point: what makes a review good? I have my thoughts on this matter, but other users, especially new users, might be put off by code-review the way I see it:
IMO, good Code review is tough love. You can only improve the code by being brutally honest. A proper review of your code often leaves you feeling bad, and that's good. Because your code is making you feel bad, you're less inclined to try and make it work. The delete and backspace keys suddenly don't seem dangerous or scary.

Sadly, sites such as these aren't very welcoming to harsh answers. You're asked to be friendly, and to keep in mind that some users are amateurs/hobbyists. This means there's a trade-off: either we prefer honesty or we are more welcoming to new members.
SE chooses the latter. For this reason I feel forced to write more verbose, overly polite and caring answers, with the occasional applogy and reassurance like "It's not all bad".
Sure, though love doesn't mean name-calling or comments such as burn it before it hatches! and leave it at that. You have to explain what is wrong, so even harsh reviews still have to show effort on the reviewer's/answerer's part. That said, questions like "assess my skill level" are hard to answer: What aspect do you focus on the most? The patterns used? The code itself? What level of knowledge does the OP actually have?

Here's an example of an assess-my skill-question. There's nothing wrong with it. If anything, it's the best of the questions I'll linked to in my original post. The OP kindly provided 3 focus-points. So I dealt with them one by one. I put everything as politely as possible, but I could've linked to the SOLID wiki page, saying "Read it, learn it, and refactor".
Instead, I brought it up several times, because putting it bluntly wasn't going to make the answer very palatable for the OP, or any of the other CR users.
Much like jslint warns you that "Using jslint will hurt your feelings", inform everyone that code-review can be nasty, hence:

\$\endgroup\$
3
  • \$\begingroup\$ Do you feel that the requirement to be polite hampered your answer to the question? \$\endgroup\$
    – Winston Ewert
    Sep 18, 2013 at 17:35
  • \$\begingroup\$ @WinstonEwert: Not in this example, but the reason why I posted my first series of suggestions is actually because they're all sort-of intertwined: because I have to be polite (which, in itself, I don't mind at all), my answers tend to be more verbose. Some people are less likely to read them (as I explained in my original post). To cut to the chase would've meant my being more blunt. If you look at the edits I made here, I didn't really add new info, but my initial answer was blunt \$\endgroup\$
    – Elias Van Ootegem
    Sep 19, 2013 at 7:07
  • \$\begingroup\$ That's why I linked to my meta question: to clarify why I feel that seemingly blunt answers are, in essence, part of good CR. Of course, I did make an effort to clarify a lot of things, but I think I wouldn't have, if the OP wouldn't have taken my harsh criticism as well as he did. I feel he took it well, in part, because I linked to my explanation of why I was being harsh, and my saying I meant it in a good way. To avoid having to do this, I thought it a good idea to let everyone know this from the get-go. That is, of course, if enough people feel the same way about harsh criticism = good \$\endgroup\$
    – Elias Van Ootegem
    Sep 19, 2013 at 7:11

1 Answer 1

Reset to default
7
\$\begingroup\$

I really have no objection to adding an entry to the FAQ warning people that good code review is tough. I can't really see how it would hurt anything.

I also must agree that good code review is harsh, blunt, and honest though not impolite.

However, there are three issues with your suggestion from my perspective:

Firstly, we've got no indication that this has been a problem. I'm unaware of anyone whose gotten upset because people have ripped their code to shreds. I've had many people express gratitude when I've gone through and told them just about every line in their was code was poorly written.

Secondly, what you say about how your posts would be different with such a FAQ entry make me worry that your posts would suffer in quality. You seem to suggest that you would have just linked to a SOLID wiki page rather then giving more detail about how exactly the code failed to meet the criteria. Your discussion seems to suggest that you think your answers could be less detailed if you didn't have to be so nice, which concerns me.

Thirdly, new users don't read the FAQ. Probably, they should, but they don't. If we put a question and answer in the FAQ about good code review being blunt/harsh they won't read it. A FAQ won't give us the opportunity to reshape how new users perceive our community.

Basically, I think you need to show more evidence that this is a problem either in how new users react or in how people write posts. We also need reason to believe whatever proposed solution you have in mind will actually help the address the issue. Right now, its not clear to me that there is a problem, and its not clear that proposed solution would address the problem.

\$\endgroup\$
6
  • 1
    \$\begingroup\$ Seconded on the thing about new users not reading the FAQ. I also don't see this (niceness and such) as a site policy per se. Generally, the answerer should be free to be as harsh as necessary, and that's it. Even if the OP's code is completely messy, the answerer shouldn't express any frustration, but assertiveness so that the OP gets the important points nailed down. \$\endgroup\$
    – Jamal
    Sep 21, 2013 at 7:56
  • \$\begingroup\$ Granted, I've but once gotten a rather blunt response to a rather blunt answer. Though I linked to the manual page, where many comments explained how, what the OP was after, could be done, he took offense at my answer. I admit: it wasn't very polite, but then: the question didn't show much effort either. I must stress, though, that I did put the effort in, regardless, but, like on SO: some people get an answer in quick and expand on it through edits. That's fine \$\endgroup\$
    – Elias Van Ootegem
    Sep 23, 2013 at 14:04
  • \$\begingroup\$ So why not allow the same answers on CR? get the quick and nasty stuff out first, because tearing code to shreds is easy. Just add a quick phrase like I will go into more detail in future edits, and let the OP wait... he/she should appreciate that CR is blunt and takes time, and words... so he/she could see quick and nasty responses as an indication: people are looking into it, and they will focus on this or that aspect of my code... this way, they can comment back quickly to say: "I know, but I wanted to focus more on my usage of OOP techniques" or something, making CR more interactive \$\endgroup\$
    – Elias Van Ootegem
    Sep 23, 2013 at 14:07
  • \$\begingroup\$ @EliasVanOotegem, if you want to discuss whether its appropriate to post and then expand, please start as its own question. I don't see anything particularly wrong with it, but it should get more people's opinions. Its just not the same issue you've brought up here. \$\endgroup\$
    – Winston Ewert
    Sep 24, 2013 at 1:35
  • \$\begingroup\$ As to the question you reference: You implied he was lazy and gave him a code snippet that didn't work. What did you expect to happen? \$\endgroup\$
    – Winston Ewert
    Sep 24, 2013 at 1:41
  • \$\begingroup\$ @WinstonEwert: I admit that my initial response was a tad "off", in the same way the OP's question showed very little effort. That's why I deleted it at first. I did some more digging (mainly on the man page to which I had linked), and edited my original answer, then undeleted it. What I should've done was perhaps remark on the OP not putting in enough effort, link to the page and include a note on the expr1 ?: expr2 short notation. \$\endgroup\$
    – Elias Van Ootegem
    Sep 24, 2013 at 7:15

You must log in to answer this question.

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