0
\$\begingroup\$

I asked this question:

Templated division by a power of 2

and did not include an #include directive before my actual code. An answer noted the #include must exists but was not part of the listed code, and went on to address the code with the unlisted #include.

Adding the #include is not a change of the code I was asking about, it is merely a clarification that I'm not using some other weird replacement of the standard library; and I made this addition with an edit. I did not change my actual code.

Another user rolled my edit back, citing this page. I read that page (again), but since I was not changing my code, I decided the edit had been legit, and re-introduced it (with a comment). The edit was then rolled back and the question lock without my comment being addressed.

I believe the roll-backs were excessive and unnecessary. Am I wrong?

\$\endgroup\$

2 Answers 2

Reset to default
8
\$\begingroup\$

The edit was a change to the code, and specifically a change to an aspect that has been mentioned in a review. If you make changes to your question, you are expected to ensure that the answers remain correct - this generally means not changing the code that's been reviewed.

There's really no need to change the code in the question anyway - any subsequent reviewer will read existing answers before starting, so will know how to fix that before adding their own observations.

\$\endgroup\$
12
  • \$\begingroup\$ BTW, I can't respond to Martin York's comment on the question (due to the lock), but I think that compilers generally make that optimisation when dividing by a constant. I don't know whether any compilers can read assert((q & q-1)==0); return p/q; and use the assertion to optimise the division to a shift - perhaps you have better knowledge there? \$\endgroup\$
    – Toby Speight
    Commented May 30, 2019 at 12:25
  • \$\begingroup\$ The edit was not a change to the code. \$\endgroup\$
    – einpoklum
    Commented May 30, 2019 at 12:56
  • \$\begingroup\$ To answer your comment: I would suppose compilers make this optimization. It's either too complicated or too specific IMO. \$\endgroup\$
    – einpoklum
    Commented May 30, 2019 at 12:58
  • 2
    \$\begingroup\$ “any subsequent reviewer will read existing answers before starting” while it is typical, I don’t feel this is always the case (given some late answers that don’t add anything new) so I would suggest modifying the word “will” to “should” \$\endgroup\$
    – Sᴀᴍ Onᴇᴌᴀ Mod
    Commented May 30, 2019 at 13:07
  • 2
    \$\begingroup\$ The edit was a change to the code - specifically, it added the #include line which had been omitted and which the review pointed out. \$\endgroup\$
    – Toby Speight
    Commented May 30, 2019 at 13:11
  • 2
    \$\begingroup\$ @SᴀᴍOnᴇᴌᴀ, I considered using "should", but was feeling optimistic... \$\endgroup\$
    – Toby Speight
    Commented May 30, 2019 at 13:12
  • \$\begingroup\$ @einpoklum Question is now unlocked. \$\endgroup\$
    – Simon Forsberg Mod
    Commented May 30, 2019 at 13:55
  • \$\begingroup\$ @SimonForsberg: Thank you. \$\endgroup\$
    – einpoklum
    Commented May 30, 2019 at 14:58
  • \$\begingroup\$ @TobySpeight: Nope, no change. The #include had always been a part of the code. \$\endgroup\$
    – einpoklum
    Commented May 30, 2019 at 16:11
  • 1
    \$\begingroup\$ @TobySpeight My knowledge on compilers is 20 years old. So I could be wrong. But a compiler can do a lot of the maths on const values (and not contexpr) at compile time. And I have seen compilers check to see if this was a division by 2 (or multiple of 2) and do shifts. BUT that is not my issue. My issue is with programmer micro optimization. You are unlikely to beat the compiler, but you could. The Problem is that your optimization just locked you into a technique that the compiler probably can't optimize and thus you are pessimizing your code in the long run as the compiler improves. \$\endgroup\$
    – Loki Astari
    Commented May 30, 2019 at 16:26
  • 6
    \$\begingroup\$ @einpoklum, the #include was not previously part of the code in the question, so was not part of the code under review. I'm sure it was always part of the code you compiled yourself (otherwise you could never have tested it), but reviewers don't get to see that, and have to base reviews only on what is presented to them. I wouldn't sweat it for a single line fix - but it does help if you post complete code that reviewers can compile and run for themselves without needing to change anything (that's not a requirement - it just help get better answers). \$\endgroup\$
    – Toby Speight
    Commented May 30, 2019 at 16:39
  • \$\begingroup\$ @Martin, that may be worth writing as an answer, now the question has been unlocked. It's certainly a valuable observation to question whether the code that's written actually has value or will just become an albatross. \$\endgroup\$
    – Toby Speight
    Commented May 30, 2019 at 16:49
3
\$\begingroup\$

The lock was introduced after the second rollback to your question. A lock is a moderator tool that is powerful to use in order to calm things down. While I probably would have used a lock in the case of a third rollback, I still support its usage here after the second rollback.

The alternative to a lock would have been to just rollback and risk a continuing rollback war. Those things are tedious and there's to my knowledge no guarantee to get a notification when a rollback happens. Therefore, using a temporary question lock is the easier option, to make everyone understand and agree and possibly discuss (in chat or on meta).

A recommendation in the current situation: Add a comment to your question stating that you forgot to add the missing #include-statement to the code in your original question. This should benefit everyone.

\$\endgroup\$

You must log in to answer this question.

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