Re: [PATCH 00/44 take 2] [UBI] Unsorted Block Images

[Theodore Tso]

On Mon, Feb 19, 2007 at 02:48:23PM +0200, Artem Bityutskiy wrote:
> I actually did not mean these patches should be included to a git. We
> have UBI git to pull from for these purposes. I basically manually split
> the UBI sources to make UBI easier to review. I should have added an
> "RFC" tag, apologies.

It made it much, much, MUCH harder to review.  Especially given that
the documentation was separated from the implementation.  As I looked
at the implementation, there was no way to look and what it was
supposed to do without flipping back to a previous e-mail message and
losing my place.  

> This reflects the way of my thinking. I see UBI as a set of units with
> defined interfaces. So I even physically split the interface description
> into files. I still think it is easier to grasp the architecture this
> way.

Speaking as someone who was coming into it cold, it actually made it
far more difficult.  Your units were too small, so that meant the
number of interfaces that were created as a result were huge!  (Around
20 _sets_ of interfaces, all of which had to be comprehended for what
should have been a relatively simple set of functionality!)

And when you create that many interfaces, it adds inertia to changing
the interfaces later on, because it's sometimes not clear how many
users of the interface there really are.  My general rule of thumb is
that if an interface only has one user, then it may be a good idea to
combine it with the user of that interface, and then make the
functions involved be a static, so that it becomes clear the only user
of that functoin is within that one file.  You can take this too far,
and to extremes it doesn't work all that well, but the UBI layer has
gone waaaaaay off the deep end in terms of functional decomposition.

Adding lots of units so you can have formal functional interfaces is
not always the right answer in terms of making code more readable or
maintainable.  And in my humble opinion, UBI is a picture-perfect
demonstration of this.

Regards,

						- Ted
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/