On Sat, Sep 19, 2015 at 05:57:23PM -0400, Michael McConville wrote:
> > What's your thinking behind this? To me this seems like a perfectly
> > rational and well motivated function to have, both for readability and
> > rather than having to repeat the same statements several times over in
> > the rest of the code, risking future calamity if a change is made in
> > one place and not the other...
> >
> > I mean, I don't know if I'm unusual, but I often find myself breaking
> > out separate functions from larger pieces of code, that may end up
> > being called just that one time in a program, just to improve
> > readability and keep things logically apart.
>
> The most important thing to note is that this diff removes seven lines.
> If you're abstracting something into a function, it definitely shouldn't
> be creating more code.
>
> When you create functions for things like if (z) errx(...); conditions,
> it makes the code much harder to read. Someone trying to figure out
> what's going on has to find the function definition and remember what it
> does. Extrapolate this for every two-line error condition, and you have
> a major headache. Such functions improve readability only if you're the
> one writing them.
>
> So, I wouldn't like this function no matter how many times it was used.
> And it only appears twice.
I do not agree. You only have to remeber "that function does sensible
error checking" and you do not have to remember at each spot which
condition is the right one.
Function are the major way of structuring code, use them.
-Otto
