Do you use IRC at all? We could chat there.
On Sat, 2009-09-05 at 09:21 -0400, Rob Hasselbaum wrote:
Steve,
Any additional input before I upload a new patch?
-Rob
On 09/03/2009 10:28 PM, Rob Hasselbaum wrote:
> Hi Steve,
>
> Thanks for the feedback. I am working on moving the patch to trunk
> as you requested. I'll update the bug when that's done. My comments
> on the rest of your feedback are inline below.
>
> On 09/03/2009 04:39 PM, Steve Ebersole wrote:
> > 1) Personally, I don't like the attribute names sql-get and
> > sql-set.
> > When I think through trying to describe and explain this feature
> > to
> > people, the terms "wrap" and "unwrap" keep coming to my
head as
> > being
> > descriptive, relevant and natural. It was really the
> > "naturalness"
> > aspect that got me with sql-get and sql-set. Other terms I could
> > think
> > of included encode/decodeAny other suggestions?
>
> I went with "sql-set" and "sql-get" because they are consistent
with
> the existing "sql-type" attribute and make sense to me, but I'm not
> married to the terms. Wrap and unwrap sound good, too. Bear in mind
> that the expressions need not be function calls, which wrap and
> unwrap might suggest. The functional tests do math, for instance.
>
> > 2) Can anyone foresee a valid use-case for allowing one of these
> > to be
> > defined w/o the other? The only thing I came up with was for
> > immutable
> > properties. So something like <column name="xyz"
> > unwrap="decode(xyz)"
> > insert="false" update="false"/>, which can also be
defined using
> > a
> > formula like <formula>decode(xyz)</formula>. The reason I ask is
> > that
> > if they should always be used together then maybe it is better to
> > enforce that in the DTD and/or binder
> >
>
> I couldn't think of a use case for a one-way conversion either. In
> fact, you could end up reading out a different value than the one
> you put in if you're not careful. But I'd regret adding an
> artificial restriction if someone came up with a legitimate use case
> next year.
>
> > 3) You renamed the getTemplate method on Selectable to
> > getGetterTemplate. Everything in the o.h.mapping package is an
> > SPI that
> > is used by many other libraries. We need to be very careful
> > about
> > changing stuff in here. I am not sure if folks bind to this
> > particular
> > method. But since this is just a cosmetic change, I think it
> > should be
> > reverted.
> >
> >
>
> Sorry, this was an oversight. At first, I thought I was going to
> need to store a setter template in addition to a getter template in
> the Column class, It turned out that I didn't need it, but I forgot
> to revert the name of the getter template. I'll fix that.
>
> > Still not done looking through the whole patch. Will try to
> > finish up
> > tomorrow.
> >
> >
>
> Thanks,
> -Rob
>
--
Steve Ebersole <steve(a)hibernate.org>
Hibernate.org