Hello Andrus,
thanks for the quick reply.
> So there are a few issues here:
>
> (1) stripping "AsList" suffix. (+1) I don't like it myself. Let's do it.
OK. This was only an 'accident' anyway since return types are not part of
the method signature and it's hard to have two methods with the same
signatures..
> (2) changing List to Collection. (-1) Lists are easier to use in the GUI.
> One thing Cayenne does already is alphabetic ordering of entities and
> such.
Yes. In some places the framework uses Lists for internal storage, in
other places it copies mostly Map.values() into a List without ordering.
If it's unordered you can just as well return a Collection and have the
client call new SortedList(xxx.getEntities()) - that's why I wrote the
SortedList (among other things ;). The whole idea was primarily motivated
by Map.values() returning Collection, and really makes a lot of
dependencies simply vanish since the duplication is gone. In 90% of all
places the get**AsList methods are simply used for iteration, and it would
make no difference to the client.
Maintaining both makes matters a lot more complicated and the stepwise
improvement harder. First you'd need the events to keep things consistent
(and you need all at once!), but you can only finish these if Modeler is
kept in sync, and that's hard to do and has big repercussions for the
rest. I always try to pick the low-hanging fruit first, and automated
refactorings are just that.
How about this:
- we keep both
- use the map.values() reference where possible instead of the expensive
get**List method
- the **List methods are kept in there for the remaining callers and still
return a copy
- when the event stuff is in place, the list can be cached and becomes
cheap.
> (3) returning immutable collections. (+0) I kind of like it, but we need
> to dig into all the consequences (especially in the modeler). I know you
> are already checking into that...
yes. It's really easy to catch:
- browse all senders of a method (Smalltalk lingo breaking through ;)
- check them one by one if they modify something, if so make them create a
copy
- most callers simply iterate, so nothing to change
> (4) Schedule: There are a few things that need to be implemented before Beta:
>
> - stored procedure support
> - Andriy's imheritance work
> - Arndt's inheritance patches (we need to make a schedule decision about
> those - anyone feels like integrating them now?)
I'm afraid I can't contribute anything to the inheritance since I just got
a new project and have a deadline to keep. I'll also finish the Config
stuff (which is mostly done IMHO).
> Beta will take at least a few more weeks (I will be on vacation in Europe
> early April, I hope we can release till then, but I don't have the
Make sure you check out our classic freedom-loving french fries :->
> confidence). So if you feel like doing naming refactoring, there is this
> time window.
OK
> Once we are in Beta I would suggest to freeze API and only fix bugs (with
> Modeler being a possible exception). This is the whole point of Beta.
yes. like I said, alternatively we can put this off until b2 but if you
think it's still some time off there's no point in wasting time. There's
little merit in 'freezing' bad API only to break it later anyway. Nobody
expects something < 1.0 to be stable.
> I am not sure I am following this. I don't see a problem with DataContext
> relying on parent's implementation. Changing DataDomain to return an
Sorry, I poorly described what I had in mind. Returning the parent is of
course completely OK, it was just an example for "fix one - get the other
for free".
> immutable list is totally OK. Renaming getDataMapsAsList -> getDataMaps in
> the interface (and all implementations) is OK.
OK
> > DataMap:
> > - rename to getDb/ObjEntities
> > - return immutable read-through Collection
> > - remove getDbEntityMap (not used elsewhere)
> > -or-
> > - remove *List methods completely
> > - rename getDb/ObjEntityMap to getDb/ObjEntities
> > - return immutable read-through Map
>
> I'd prefer keep the (renamed) list method, and keep the Map methods
> (returning immutable maps I guess).
Well this won't give any benefit except for the name cleanups, since then
the Lists have to maintained, and like you said below this can only be
done reasonably well when the event stuff is in place. The *List methods
don't say whether the entities are ordered, and I don't think they
actually are (will check). But even if they are, the values() will be too.
Basically this is the same problem as above: >90% of the callers of
get**List just iterate over it, instead using get**.values().iterator
would not change a thing for worse.
> > DataContext:
> > - instead of calling clearFlattenedUpdateQueries from ContextCommit make
> > DC clean up itself after commit; make method private or remove
> > completely
>
> +1. If ContextCommit can only rely on public API in DataContext, this
> would be great.
OK (the flattenedInserts and flattenedDeletes would be still there)
> > ObjRelationship:
> > - rename getDbRelationshipList to getDbRelationships
> > - return immutable List
OK I guess?
> > - cache reverseRelationship
> >
> > DbRelationship:
> > - cache reverseRelationship
>
> We can only cache stuff when we implement reliable update mechanism via
> notifications.
yes. I just re-checked and saw that it was more complicated than on first
sight; will be easy once EntityEvents are flying around, so back burner
for that one.
sorry for writing so much :-)
Holger
This archive was generated by hypermail 2.0.0 : Thu Mar 13 2003 - 14:22:42 EST