db-torque-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thomas Fox <Thomas....@seitenbau.net>
Subject Re: BasePeerImpl, was: Re: RFD: RecordMappers, Peers and MapBuilders
Date Wed, 01 Aug 2012 08:40:57 GMT


Thomas Vandahl wrote:

> On 21.07.12 15:04, Thomas Fox wrote:
> >> - I'd inject a RecordMapper<T>, the database name and a TableMap on
> > creation of the
> >> instance (setters and/or constructor)
> > +1
>
> Ok, this basically reduces the importance/usability of BasePeer but
> increases the usability of BasePeerImpl. Maybe, BasePeer can go some day.

It still contains the static wrappers to the BasePeerImpl methods. They
should stay there, so it will not go waway

> ...
> >> - Provide some kind of default record
> >> mapper/default record to make calls like in doSelectVillageRecords()
> >> work again.
> > I do not understand this. You want to put the results in a map ?
>
> Yes. More or less. There are cases where this is necessary, at least in
> the code I maintain.

Have you seen org.apache.torque.om.mapper.CompositeMapper ? This seems to
be a bit like what you need, although it does not provide type selection
from the table map. Feel free to add another RecordMapper class to provide
that functionality.

> >
> >> - Remove the record mapper and the table map from the
> >> various method signatures
> > +1
>
> What I did was to add the doSelect() methods from the generated
> XXXBasePeerImpl but leave the methods in. They are explicitly used by
> the doSelectJoin...() methods. Maybe we can make them protected.

Users might also want to have access to the methods where you can pass a
RecordMapper to make partial selects (read not all columns) and similar. So
leavintg the methods in is ok.

> >> - Remove the then obsolete methods in the
> >> generated PeerImpls.
> > +1
>
> The result is attached.

Looks good to me. Please commit (I'm afraid there haven already been
changes since you started, so if it is not a problem please consider these
changes also)

> And what remains from BaseAuthorPeerImpl, for
> example. Actually, only the buildXXX() methods and the fillXXX() methods
> must be generated, *if* we could make BasePeerImpl abstract

No problems with that.

> or provide
> some default implementation that throws a RuntimeException of sorts.

I'd rather not do this. If Torque is used the typical way there is no need
to instantiate BasePeerImpl itself, so no need to hack a dirty solution.

> >> Question: -
> >> Why do we have deprecated methods in a class that didn't even exist
> >> in Torque 3.3? Is it just for keeping the old Criteria object alive?
> >
> > It could allow for a soft transition form the old to the new criteria
> > class.
> > people just change the import but use the deprecated method...
> > but maybe this scenario will not happen in practice, no idea.
>
> I'm afraid this creates more confusion than it solves. The 4.0 release
> should be expected to be incompatible, and then it's just a matter of
> Ctrl+O in Eclipse to resolve that changed import. Just my 2 cents.

Ok, if there are no objections from others, I'll remove the deprecated
methods from the new Criteria objects.

    Thomas


---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org


Mime
View raw message