8
\$\begingroup\$

I stumbled upon this question in the reopen queue, and looking at it I could find reasons to leave it closed, so I voted [leave closed].

Then I was curious about what made it gather so many reopen votes, and went to the review history, and from there to the post itself.

It was closed by a trusted user and a moderator. So I read the question again. Then again, and again.. and came to the conclusion that the only solid reason I had to vote to close, was that it was a . So I downvoted the post, voted to reopen, and left a comment:

I voted to leave this question closed in the review queue, because it's a comparative review showing 3 distinct approaches to a problem that isn't presented in full context. Then I came here and voted to reopen, because for now, it's community consensus that comparative reviews are on-topic. However I downvoted it, because I don't agree with this, and it remains a borderline question, but in all fairness I have to lean on the "reopen" side of the fence. It would be much better to include one of these snippets as part of actual code that's using it, and then voicing your concerns about it.

I don't know if that question was rightfully reopened. I'm biased against the format being used, but I find "hypothetical code" could be invoked for closing it - but it would be unfair to that user to re-close that question just with a different close reason, and comparative reviews are supposedly on-topic.


I don't like such comparative reviews. I find they make poor questions, more than often include very little context (if any), and in my own humble opinion, even with full context, boil down to a roundabout way of asking "what's the best practice regarding X?", which is explicitly off-topic on this site.

I've seen a number of regulars mention they don't like comparative reviews either.

So I'm probing the community here: is on-topicness of comparative reviews still a consensus? What are the arguments for it, compared to the arguments against it?

\$\endgroup\$
  • \$\begingroup\$ Off topic: Hypothetical Code. These still routinely get my downvote. \$\endgroup\$ – RubberDuck Jul 2 '15 at 22:56
  • 1
    \$\begingroup\$ Comparative reviewers are the worst questions on Code Review. They miss the point. They're the only questions I downvote more quickly than LOLCODE. \$\endgroup\$ – nhgrif Jul 3 '15 at 16:15
8
\$\begingroup\$

We have had comparative review debates several times in The 2nd Monitor since I joined around last November (November 2014, for ease of future readers' reference). There are also a few other questions on meta about comparative reviews and their limits, including this. I think now would be a good time to decide to accept them permanently or ban them altogether.

I formerly was a proponent of comparative reviews, but have since changed my mind. This is why:

  1. If you have a specific requirements for the code to fulfill, you should test it yourself, not ask the community to test it for you. Maybe you need the project to be fast - then profile the code. Maybe you need it to be resource cheap - then profile it.
  2. If both code sections appear to be the same, then pick one that looks better and post just it on CR. People here are the opposite of dumb, and probably know about both techniques. Writing code isn't an A/B multiple choice exam, and quite possibly, the best solution is C. If you wish, you could say that section 'A' doesn't look write (or some other reason) and you tried solution 'B' there, but it doesn't appear right either.

Thank you to @nhgrif and other regulars for these ideas.

\$\endgroup\$
5
\$\begingroup\$

I have been an outspoken critic of comparative reviews. In my estimation, only one of the alternatives can be considered "real code in a real project". The other alternatives are not being used..... so are not "real".

That is a fairly dubious concept though, I admit.

My preference, and what I have tried to do recently, is to re-phrase comparative questions to be preference-with-backups questions. Consider this one: Reorganizing inventory items into a data structure

The original version was phrased as a comparative question. It was also mostly off-topic.... but the point is that the final version selected one of the alternatives, and presented it as the only code to review, then had the other code as background information on what alternatives had been tried.

The OP got the sort of review they wanted anyway, and the question is more clearly on topic. Only one of the alternatives was reviewed.

My suggestion for comparative posts is that the OP should be required to select the alternative they "use", and to then review that alternative, perhaps referencing the other alternatives as 'fodder' for thought.

In the specific example question in this post, it could easily be phrased as:

This is my code, what do you think, I have also tried these other variants (that are not for direct review) but I want to know if there's still stuff I can do better?

\$\endgroup\$
  • \$\begingroup\$ Interesting thought. How would you handle "old" questions and new questions that don't have that mandatory selection? \$\endgroup\$ – Vogel612 Jul 2 '15 at 20:06
4
\$\begingroup\$

I don't like comparative reviews, because they tend to be significantly harder to deal with. With "normal" reviews, your task is to suggest improvements. With comparative reviews, there is an added dimension of the comparison, which can be highly complex. Many times the answer is "neither is good, choose a 3rd way". That's noticeably more effort than other kind of questions.

Ideally, I would much prefer if OP picked the very best possible implementation he can come up with, and pasted that for code review. So much simpler.

I don't like such comparative reviews. I find they make poor questions, more than often include very little context (if any)

For the record, I don't think that comparative reviews inherently don't include much context. At least I don't have that experience. They can be poorely formed or well formed just like any other category. On the other hand, they are hard to write well, so they are often voted to close, or downvoted out of frustration.

Although these questions are harder to answer and harder to deal with than others, I still agree with the meta post you linked: they can be on-topic, if well written. I'm afraid we have to live with these.

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

The problem with this question is not that it's a comparative review. Let's compare (haha) the current question…

Filtering/mapping strategies

This is real JavaScript/Lodash code used in a project I'm currently working on, but actually this particular question bothers me quite often regardless of the language. Say, I need to generate some data relying on some filtered out piece of other date.

Here are three approaches:

Implementation 1, Implementation 2, Implementation 3

Which one is to better for making an idiom? I caught myself using them interchangeably.

… with what I consider to be a more concrete question:

Extracting relevant key-value pairs to represent an activity

My app has a form in which users describe attributes of their favorite activity. Given a userData object, I need to extract a list of just the key-value pairs that the options object says are relevant.

I'm having trouble deciding between the following three implementations:

Implementation 1, Implementation 2, Implementation 3

My primary concern is readability; performance is somewhat important too.

The second version would be a fine code review question, even if it asks for a comparison. I'm not sure what the first version is; it contains some slippery phrases that make it more like a general best-practices question than a review of real code, even if the author insists that it is real code:

  • "strategies" in the title. The title also looks like it describes a general topic than a specific task; this is a warning sign mentioned in How to Ask.
  • "regardless of the language"
  • "Say, I need to …" rather than "I need to …"
  • "Here are three approaches" rather than "Here are three implementations"

That said, there is reviewable code. I don't believe that the question is far enough off-topic for me to close it as a moderator, as long as the original poster refrains from insisting that the code presented actually stands for a whole genre of similar code.

\$\endgroup\$

You must log in to answer this question.

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