9
\$\begingroup\$

This question was put on hold for being off-topic:

https://codereview.stackexchange.com/questions/58091/why-is-it-suggested-to-replace-temp-with-query

To refactor the code, it is suggested here to replace Temp with Query, for example in following code, basePrice should be extracted to a method. I am wondering why is it a good practice? If I keep the temporary variable the equation will be calculated once rather than three times.

double basePrice = _quantity * _itemPrice;
if(basePrice > 1000)
    return basePrice * 0.95;
else
    return basePrice * 0.98;

Convert to

if(basePrice() > 1000)
   return basePrice() * 0.95;
else
   return basePrice() * 0.98;
....
double basePrice(){
   return _quantity * _itemPrice;
}

This was closed on the grounds of:

"Questions must involve real code that you own or maintain. Questions seeking an explanation of someone else's code are off-topic. Pseudocode, hypothetical code, or stub code should be replaced by a concrete example."

I think the question is different from the "someone else's code", or "hypothetical code" categories. It's about a not-so-clear-cut practice recommended by a book, and something that can easily come up during a code review session in real life, in real projects, where I'll have to take a stand and accept or reject the proposed change. It will be nice to have a public discussion on this and find out what our community recommends.

The question falls well within the two top items in the "Purpose of a Code Review" of this related post:

  • Conformity and maintainability - does the code conform to the standards and expectations of your peers... [...]
  • Performance and efficiency - are there ways to make the code go faster, or do what its doing more efficiently

So the question is, do you still think it's off topic? If not then let's reopen, and let's take a stand.

UPDATE

This kind of question could be labeled as "refactoring technique". It's a practical issue that can easily come up in real life code review, and it deserves an answer.

\$\endgroup\$
9
\$\begingroup\$

The question simply isn't Code Review material:

  • The amount of code is barely sufficient to give us context for a review.
  • The question asks for an opinion on a particular programming practice, but a good code review would assess so much more: braces, naming, whether the problem deserves better abstraction, etc.
  • The question seems generic and hypothetical in nature. That criterion is sometimes hard to assess, but…
  • In this case, it's clearly not the poster's own code or even a real project; it's a hypothetical example that came from someone else's website.

Generic stuff like that belongs on Programmers SE, not Code Review. I didn't migrate it because I didn't feel like it was a meritorious question for Programmers SE either, as currently written. There needs to be enough detail to prevent it from being closed there as "primarily opinion-based".

\$\endgroup\$
8
\$\begingroup\$

Point number four seals it.. It's not the OP's code. The question is off topic, but I would like to offer some counter arguments.

  • Barely enough context is still enough context.
  • Even though OP had a very specific question, reviewers are free to comment on anything in the code.
  • It certainly doesn't look like hypothetical code does it? Indeed that is hard to ascertain. If OP had not been so honest, and posted the reference link, this would have slipped through the cracks.

Just food for thought.

\$\endgroup\$

You must log in to answer this question.

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