Re: [PATCH] PAG support only

[Miles Bader]

David Howells <[e-mail]> writes:
> So you'd rather have:
> 
> 		if (pag > 0)
> 			return vfs_join_pag(pag);
> 		else if (pag == 0)
> 			return vfs_leave_pag();
> 		else if (pag == -1)
> 			return vfs_new_pag();
> 		else
> 			return -EINVAL;
>
> Than:
>
> 		if (pag > 0)		return vfs_join_pag(pag);
> 		else if (pag == 0)	return vfs_leave_pag();
> 		else if (pag == -1)	return vfs_new_pag();
> 		else			return -EINVAL;
>
> When the former is _far_ less readable at a glance?

Keep in mind that `readable' also means that someone used to the
kernel's coding style, but not to your particular portion, should
naturally be able to follow the code with as little trouble as possible.
While I agree that your second example is more clear once one sees
what's going on (and it's certainly prettier in some sense), the `shape'
is unusual, so it takes some adaptation on the part of the reader to
make this jump, and that's something to be avoided if possible.

My personal feeling is that (1) there are exceptional cases where the
rules should be broken, and (2) but not that many.

These exceptional cases usually stick out like a sore thumb, and if
there's any doubt, it's probably not one.

-Miles
-- 
We live, as we dream -- alone....