Messages in this thread | | | Date | Wed, 18 Jun 2008 12:06:24 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH] shm: Remove silly double assignment |
| |
* Neil Horman <nhorman@tuxdriver.com> wrote:
> > Not silly, really. Look: > > > > err = -ENOMEM; > > if (expr1) > > goto out; > > > > err = -ENOMEM; > > if (expr2) > > goto out; > > > > each of these two units is a separate, self-contained clause. > > Removing the second assignment to `err' breaks that separation and > > will make one clause undesirably dependent upon the other. > > yes, I see that keeping the second assignment lets you insert code in > the middle without having up to update afterwards. I just don't think > the redundant assignment is worth keeping just for that purpose.
cleanups is all about having familar, robust, predictable, self-contained patterns of code that are easy to recognize, easy to review and easy to keep working well.
In this case we've got an admittedly boundary case: removing the technically redundant piece of code, while a valid cleanup at first sight, makes the code less robust against future changes.
Ingo
| |