Re: svn commit: r665699 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/table/ layoutmgr/ layoutmgr/inline/ layoutmgr/list/ layoutmgr/table/

2008-06-09 Thread Jeremias Maerki
On 09.06.2008 18:46:23 Adrian Cumiskey wrote: > After looking more closely at this I think I may have been a little hasty in > my praise and Jeremias > has a point, but Max's sentiment is a right one. I don't disagree. > On the subject of readability, there is much cleaning up and refactoring t

Re: svn commit: r665699 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/table/ layoutmgr/ layoutmgr/inline/ layoutmgr/list/ layoutmgr/table/

2008-06-09 Thread J.Pietschmann
Jeremias Maerki wrote: Frankly, I'm less than thrilled. I appreciate the good will behind this but I'd have appreciated some advance warning, too. My concern is that: -ListElement last = (ListElement)contentList.getLast(); is much easier to read and write than: +ListElement last

Re: svn commit: r665699 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/table/ layoutmgr/ layoutmgr/inline/ layoutmgr/list/ layoutmgr/table/

2008-06-09 Thread Jeremias Maerki
On 09.06.2008 17:45:51 Andreas Delmelle wrote: > It never hurts to check with the other devs > before committing. If I find myself in such a situation, I'm inclined > to attach the patch to a Bugzilla entry first, and give everyone a > chance to comment in on the proposed changes. If no feed

Re: svn commit: r665699 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/table/ layoutmgr/ layoutmgr/inline/ layoutmgr/list/ layoutmgr/table/

2008-06-09 Thread Jeremias Maerki
Thanks, Max, I guess I can live with a few helper methods, for example, in ElementListUtils. Code readability and avoiding trivial coding mistakes is important, especially in such a complex package as "layoutmgr". On 09.06.2008 18:56:52 Max Berger wrote: > Jeremias, > > actually I had the same re

Re: svn commit: r665699 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/table/ layoutmgr/ layoutmgr/inline/ layoutmgr/list/ layoutmgr/table/

2008-06-09 Thread Vincent Hennebert
(I had been writing this before I received Max’s answer. Still...) >From a general point of vue I agree that declaring a LinkedList makes sense, when you want to stress the fact that you’re working with a sequential structure with which it’s cheap to add/remove/access elements at the beginning or

Re: svn commit: r665699 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/table/ layoutmgr/ layoutmgr/inline/ layoutmgr/list/ layoutmgr/table/

2008-06-09 Thread Andreas Delmelle
On Jun 9, 2008, at 18:46, Adrian Cumiskey wrote: After looking more closely at this I think I may have been a little hasty in my praise and Jeremias has a point, but Max's sentiment is a right one. On the subject of readability, there is much cleaning up and refactoring that needs to be d

Re: svn commit: r665699 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/table/ layoutmgr/ layoutmgr/inline/ layoutmgr/list/ layoutmgr/table/

2008-06-09 Thread Max Berger
Jeremias, actually I had the same reservations (especially about the "last element", and if you have a real problem with this feel free to revert this patch. Maybe we could write a simple helper method instead? static Element getLastElement(List l) ? Would that disperse your concerns? Why

Re: svn commit: r665699 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/table/ layoutmgr/ layoutmgr/inline/ layoutmgr/list/ layoutmgr/table/

2008-06-09 Thread Adrian Cumiskey
After looking more closely at this I think I may have been a little hasty in my praise and Jeremias has a point, but Max's sentiment is a right one. On the subject of readability, there is much cleaning up and refactoring that needs to be done to make the code base more simple and easy to under

Re: svn commit: r665699 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/table/ layoutmgr/ layoutmgr/inline/ layoutmgr/list/ layoutmgr/table/

2008-06-09 Thread Andreas Delmelle
On Jun 9, 2008, at 16:40, Jeremias Maerki wrote: Frankly, I'm less than thrilled. I appreciate the good will behind this but I'd have appreciated some advance warning, too. My concern is that: -ListElement last = (ListElement)contentList.getLast(); is much easier to read and write t

Re: svn commit: r665699 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/table/ layoutmgr/ layoutmgr/inline/ layoutmgr/list/ layoutmgr/table/

2008-06-09 Thread Jeremias Maerki
Frankly, I'm less than thrilled. I appreciate the good will behind this but I'd have appreciated some advance warning, too. My concern is that: -ListElement last = (ListElement)contentList.getLast(); is much easier to read and write than: +ListElement last = (ListElement) content