8
\$\begingroup\$

This is very closely related to my meta SO question asking Are DRY questions on topic?. Most of the questions over on SO asking for DRY are not suitable for CR and should not be migrated.

In my mind, CR is the place you go when you think your code is production ready. If you know that your code is not DRY and you want it to be, isn't that almost like having broken code?

\$\endgroup\$

2 Answers 2

Reset to default
19
\$\begingroup\$

Are reviews that ask us to DRY their code within the spirit of CR?

Yes.

If you know that your code is not DRY and you want it to be, isn't that almost like having broken code?

Not the same at all.

Just because you have a feeling that your code doesn't adhere to DRY, doesn't mean that others will agree with you. Others can still say "It's not as bad as you think" and similar.

I see not much technical difference between asking the following:

  • Can you please help me DRY up this code?
  • HELP ME SHORTEN MY CODE!! OMG!!
  • It feels like this code is longer than it needs to be, can it be shortened?
  • Please review any and all aspects of the code, but I am primarily concerned with code length

One might be more friendly than the other, and if someone asks nicely, you might be more likely to review it. In terms of scope and on-topicness, however, it really ends up being about the same thing.

\$\endgroup\$
4
  • \$\begingroup\$ I have seen one review request that basically boiled down to "I couldn't figure out how to use a loop to DRY up this code so someone write it for me". Still on-topic? \$\endgroup\$
    – cimmanon
    May 31, 2015 at 13:07
  • 4
    \$\begingroup\$ I think so @cimmanon. I mean, there are a lot of those kinds of questions here. I can't think of a compelling reason to put them on hold. If we did, the "I think X may be a solution, but what I tried didn't work" would just get removed, the question reopened, and then someone leaves a review "X would fix this" without an example. At this point, OP is frustrated because they thought as much already. TL;DR: Making those off topic would harm the site more than help. \$\endgroup\$
    – RubberDuck
    May 31, 2015 at 14:26
  • \$\begingroup\$ @RubberDuck "I tried X but it didn't work for reason Y" sounds a lot like a question for StackOverflow, not CR. \$\endgroup\$
    – cimmanon
    May 31, 2015 at 14:29
  • 4
    \$\begingroup\$ You're right @cimmanon. It does, and it's likely those questions are on topic for either site. However, there is (and always will be) some over lap between sites. Being on topic on one site will never make it off topic on another. \$\endgroup\$
    – RubberDuck
    May 31, 2015 at 14:41
5
\$\begingroup\$

I think code review is about making constructive comments and recommendations about the posted code. Providing example code or suggesting alternative implementations should be optional. Questions asking for code are walking a fine line, bordering on GIMME THE CODEZ style. By the very nature of "code review", the poster should bring the code, not the reviewer.

In the spirit of DRY-ing up, an answer could include suggestions of specific techniques, such as:

  • Extract repeated pieces of code to a helper method
  • Extract repeated magic values to constants
  • Extract varying elements to a table, and use that table to determine the path of execution
  • Other ideas

Answers may optionally include example code that might help the OP, but this should not be a requirement, and I dislike when an OP asks specifically for code.

The first two techniques above are quite trivial to execute. Questions that ask to provide code for such treatment typically haven't done their homework, and effectively fall in the GIMME THE CODEZ category. I downvote + vote to close those.

\$\endgroup\$

You must log in to answer this question.

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