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 than:
+ ListElement last = (ListElement) contentList
+ .get(contentList.size() - 1);
When working with element lists constructs like the above are
everywhere.
A linked list IS the most efficient data structure for element
lists. I
generally support using the generic type instead of the specific class
(i.e. List instead of ArrayList), but LinkedList is adding some often
used methods for element lists which improve code readability. I'm
curious what other committers think about this.
Unfortunately for Max maybe, but I agree with this assessment.
In general, I have no objections to switching to the interface where
it is applicable/beneficial. As a rule of thumb, I'd reserve this
approach for any part/class in FOP that has potential use for
external libraries.
Don't confuse this with simple 'public' visibility: a lot of methods
are made public, because they need to be accessible from within other
FOP sub-packages, but that does not mean that they are part of the
public API. Similarly, some methods may be marked as 'protected', but
still they would be visible to subclasses outside of FOP. So, this
needs to be determined for every occurrence separately.
For anything that remains FOP-internal, using concrete
implementations does not only improve code-readability and -
writability, but IIC, it also generates more efficient byte-code in
the end. At run-time, the interpreter would spend slightly less time
on determining the actual implementation of a method to be used.
For the above example, the interpreter originally only needed to
check the available implementations of getLast() (defined in the
concrete class LinkedList), while after the change, it would need to
check those of get() (defined in the List interface). Checking for
the implementations of interface methods is known to be slightly more
demanding than for class methods.
Also, if we always know that the concrete type of the List in
question is a LinkedList, index-based access is generally taken to be
a bad idea.
list.getLast() is likely to outperform list.get(list.size() - 1),
especially if the lists grow large....
Painful lesson, maybe, but try to be mindful about changes across
many classes/packages, especially if they concern more than just
documentation or styling. 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 feedback comes in
after a few days, you can reasonably assume that the other committers
are OK with it.
Cheers
Andreas
