On Thu, May 02, 2013 at 09:05:01AM +0200, Johannes Sixt wrote:

> > I figured the lengthy description in the commit message would be
> > sufficient,
> It's absolutely sufficient *if* one reads the commit message. In this
> case, though it goes more like "this function should be trivial, and it is
> -- up to this if statement; what the heck is it good for?" and the reader
> is forced to dig the history.

What I really didn't want to do was to rewrite the whole rationale
again. I think something like:

   * Move the found object to the front of the collision chain;
   * this is a pure optimization that can make future lookups cheaper,
   * assuming there is some time-locality to which objects are looked
   * up.

would be enough, and interested parties can go to the commit history (I
tend to go there pretty rapidly if there is a chunk of code that sticks
out, but I guess not everybody does).

> BTW, do you notice that the function is now modifying an object (the hash
> table) even though this is rather unexpected from a "lookup" function?

I think this is fine. The function is conceptually constant from the
outside; callers don't even know about the hash table. They just know
that there is some mapping. It's similar to the way that lookup_commit
will lazily allocate the "struct commit". The callers do not care
whether it exists already or not; they care that at the end of the
function, they have a pointer to the commit. Everything else is an
implementation detail.

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to