Hi Stephen, Rich, > Would it be possible for you to refactor to remove > UnmodifiableArrayListIterator? Thanks ;-)
No prob. Please find attached updated patches & files as per your
suggestion. The comments for each file are as per the previous message
except:
- ArrayIterator.patch : changes as before, except that the "length"
field has been renamed to "endIndex", which is a far better reflection
of what the field does. This cannot break any code as the field was
private up to this point.
- ArrayListIterator.java : now directly extends ArrayIterator.
> IteratorUtils now has an unmodifiableListIterator(Iterator) method
> that wraps iterators to make them unmodifiable. I would prefer to
> keep this as the only way to make an unmodifiable iterator.
I believe there is actually a strong case for this class, which I'll
come back to at some point maybe after xmas. But it's gone from the
hierarchy now.
> Also, I am very uncomfortable with ArrayListIterator being a subclass
> of UnmodifiableArrayListIterator. There is no 'is a' relationship
> here, and worse, if I declared a method to take in an
> UnmodifiableArrayListIterator it would accept a modifiable one which
> would probably not be what I want.
Again, the UnmodifiableArrayListIterator is gone, so this is a bit
academic. But for the hell of it ;) Let's suppose we decided that we
did want to have both an ArrayListIterator and an
UnmodifiableArrayListIterator. The classes are almost identical, apart
from the #set method, which is functinal in ArrayListIterator and
throws an UnsupportedOperationException in UnmodArrayListIterator The
first thing that popped into my head was to have UnmodArrayListIterator
extend ArrayListIterator. I was going to submit the classes in that
state, but as I was reviewing the code, the 'extend' keyword caught my
eye. UnmodArrayListIterator doesn't really extend the functionality of
ArrayListIterator, unless one means 'extend' in the sense of 'restrict'
;) The UnmodArrayListIterator provides *less* functionality, and it is
the ArrayListIterator that extends the functionality that
UnmodArrayListIterator provides.
So, I rearranged the hierarchy, with ArrayListIterator 'extending'
UnmodArrayListIterator to support the #set operation, and I submitted
this.
But after your comments, I do see that there is a problem with the
structure, and it is tied in with the 'is-a' relation, but not in the
quite so obvious way. It is not always important that the 'is-a'
relationship holds. This is the case when the parent class is not
really part of the public interface per se. For instance, ArrayList
extends AbstractList, but it is not the case that ArrayList 'is-a'
AbstractList. But this isn't really a problem when the parent class is
a base implementation that implements an interface type. So, it is ok
to have hiearchies of which this is an (idealized/edited) example:
AbstractList ---- implements ---> List
|
--------------
| |
ArrayList LinkedList
This is ok because the public interfaces use the 'List' type, and the
existence of the AbstractList is not really of interest to the
end-user. Now take the case of the ArrayListIterator and the
UnmodArrayListIterator. It had been my thought that the primary useage
of these classes would be via static methods in IteratorUtils:
IteratorUtils.arrayListIterator(array) : ListIterator
IteratorUtils.unmodifiableArrayListIterator(array) : ListIterator
Since both of these operations return the interface type
('ListIterator'), *initially* it would appear that it doesn't really
matter to the end-user what the hierarchial relationship (if any)
between the two classes is. So, the hierarchial relationship is really
only of 'academic' design interest to the developers of the classes.
Both of these relationships have problems:
UnmodifableArrayListIterator
^
| // violates 'is-a'
|
ArrayListIterator
and also:
ArrayListIterator
^
| // violates 'extends' (maybe)
|
UnmodifableArrayListIterator
I chose the latter design: my reasoning was that it was better to
violate 'is-a', since the relationship was not being made public
(because of the use of the static factory methods), and it is generally
ok to violate 'is-a' in circumstances as per AbstractList just
discussed. However, as Stephen pointed out (thanks!), this is a bad
choice, because: > if I declared a method to take in an >
UnmodifiableArrayListIterator it would accept a modifiable one which >
would probably not be what I want. So, what to do? This might be design
overkill, but I think the best structure is probably as per the List
case:
AbstractArrayListIterator ---- implements ---> ListIterator
|
------------------------
| |
ArrayListIterator UnmodifiableArrayListIterator
where AbstractArrayListIterator would declare the #set method to be
abstract. Anyway! I hadn't meant that to be so verbose ;) Do any of you
hardy souls who've read down this far have any
thoughts/insight/corrections?
- Neil
commons-collections_patch_20021211_neilotoole.zip
Description: commons-collections_patch_20021211_neilotoole.zip
-- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
