On 17.03.2007 01:37:16 Vincent Hennebert wrote:
> Hi,
> 
> There are things unclear to me in the addAreasAndFlushRow method of the
> RowPainter class. I hope someone can shed some light:

I wrote this a long time ago. Let's hope I can dig some of it out again.

> - why is a Map used to store y-offsets of rows? That seems to indicate
>   that rows are not added in a sequential order, or that there could be
>   hole between them, which sounds unlikely to me.

Frankly, I don't know anymore. Bad documenting on my part. The main
reason I think was header and footer positioning because you first get
the headers then you move to the body and finally do the footers. Since
the areas generated by the table are reference areas they each have
relative positioning (X and Y) inside the table.

You can easily disable the offset map and see if you can make it run
with only the yoffset variable. Don't have time to try it myself ATM.
Sorry.

> - there is one condition that I don't understand on l.204: in case the
>   primary grid unit at the given column isn't null, then the
>   corresponding areas are added only if:
>   - forcedFlush == true, or
>   - the end of the cell is reached on the current row AND (the current
>     grid unit is null or is the last in the row-spanning direction).
>     What's the purpose of that second member of the and?

This is for situations like this:
- an empty grid unit has to be painted
  - a cell could be empty (i.e. not providing any positions)
  - a cell that is broken over multiple pages has already contributed
all its content on the previous page and does not contribute any new
positions on the current position so we have to make sure the cell is
painted all the same.

Grid units might not have been generated when there are no borders to
paint.

That's basically what the comment under line 204 says. Again you can
comment some of the if and run the table test cases to see what happens.

> - also, inside the if branch, in case the primary grid unit was null,
>   then the primary grid units from the current grid unit (i.e., on the
>   current row) is retrieved if:
>   - it does not correspond to an empty cell;
>   - the cell doesn't span in the column direction
>   - it is the last grid unit
>   Why such conditions, and can the primary grid unit still be null after
>   that (as implied by the if l.223)?

I'd have to dive deeper into this one.

> There seems to be some careful selection of the cells to be painted,
> which is still a bit cryptic to me...

What I can offer is to allocate some time next week (Wed or Fri) to
better comment (and possibly refactor) the code there. At the time of
writing I was simply focused on making the test cases work. I obviously
spent too little time better documenting some of the complex aspects
there. Maybe some of the code can be simplified. That's the problem if
you work towards making tests work: you simply stop when all tests pass
and you don't go back to refactor. Bad excuse, I know. But I was happy
to make the code work in the first place.


Jeremias Maerki

Reply via email to