On 22/07/2011, at 11:55 AM, Adam Murdoch wrote:
> Looks good. This is a really nice change to the DSL, and I'm tempted to push
> it into m4.
Cool. This was probably one of the hardest changes to a codebase I've ever had
to make.
> We should probably rename TaskCollection and PluginCollection to TaskSet and
> PluginSet.
I do think there is value in being very explicit that these things are sets.
There are also some other ambiguous names in this area I'd like change, the
main one being NameDomainObjectContainer as its name doesn't provide any hint
about it's rules behaviour.
I'm just going to do what I think is acceptable here, and trust that if I
overstep someone will pull me up.
> I think it might be good to get rid of these specialisations at some point.
> They don't really add much value.
> Perhaps we should deprecate TaskCollection.whenTaskAdded() and
> whenTaskRemoved() and PluginCollection.whenPluginAdded() and
> whenPluginRemoved().
I wonder if it makes sense to change the methods to whenAdded and whenRemoved
then?
> Also, do we want to deprecate DomainObjectCollection.getAll() and findAll() ?
Yes, I'm doing that now. It breaks a lot of tests due to JMock expectation
failures and output checks due to the deprecation warnings, but yes they are
going.
> I guess at some point (not now) we should think about turning
> NamedDomainObjectSet into a Map.
I'm not sure about this. I think these things are just sets of things that have
an implicit name. It already behaves like a map (e.g. items as properties). The
one key difference is that with the set approach, the name is inherent to the
object (see the new Namer interface). I think this has benefits and is actually
logically more correct I think.
> However, it would make sense for ResolverContainer, which extends
> NamedDomainObjectSet, to also extend java.util.List as it's an ordered
> sequence of resolvers, and the order is significant. So, we could have
> something like:
>
> interface NamedDomainObjectMap<T> extends Map<String, T>, Iterable<T> {
> DomainObjectSet<T> values()
>
> // analogs of all the methods on DomainObjectCollection - perhaps extract
> a super interface
> void all(Closure action)
> void all(Action<? super T> action)
> ...
> }
>
> interface ResolverContainer extends NamedDomainObjectMap<DependencyResolver> {
> DomainObjectList<T> values()
>
> // analogs of methods on List
> DependecyResolver getAt(int index)
> ...
> }
If we drop the idea of these things being maps as above, we end up with
DomainObjectList extends DomainObjectCollection, then NamedDomainObjectList
with very little work.
--
Luke Daley
Principal Engineer, Gradleware
http://gradleware.com
