[kronolith] Re: Re: Reimplement column spanning in DayView andWeekView(commit please)

[Chuck Hagenbuch]

Quoting Jeroen Huinink <j.huinink@wanadoo.nl>:

> I suggested a replacement of the algorithm because I think the code is
> unreadable and therefore errorprone. I have another scenario that will
> break the current algorithm. As you said before you had a lot of trouble
> getting kinks out of the code and they are still there. It therefore seems 
> to me that there is a fundamental problem and you are making things a lot 
> harder than they have to, with only a little gain. That is why I 
> implemented a simple "fixed" matrix approach.

I've just committed some more fixes/changes which fix this example that you
gave. The code is also now thoroughly commented, and I've made a few
optimizations as well.

In the process of fixing this, I did try your implementation. For one thing,
there are no comments in your implementation either, which made it difficult
for me to follow and figure out exactly what it was doing. Also, I found a
few cases that it didn't handle as well as the current code, and one case
that broke the weekview very badly (with a single event, 1am to 1am).

In my opinion, it is better to continue debugging the current algorithm
because it is:

1. Now fully documented
2. Corrected for all known cases, including years of testing on all kinds of 
   event combinations.

I appreciate the work you put into trying to fix the problem, but I think
we're better served going forward with the code as it is now. Please do
provide other examples that break the code, if you find them, and I hope you
continue contributing the side-by-side code - and hopefully more in the future!

> The approach that I originally tried to improve the current algorithm was
> through working with the "least common multiple" instead of the "max" to
> determine maximum span. But I couldn't get this to work in the time I
> gave it, although I still think that is a worthwhile approach (if you want 
> to keep what you have now). Maybe somebody more familiar with what we have
> now can give it a try.

The code does use the least common multiple, actually. It wasn't getting it
in all cases before, but my previous fix took care of that.

-chuck

--
Charles Hagenbuch, <chuck@horde.org>
"People ask me all the time what it will be like living without otters."
 - Google, thanks to Harpers