Won't there be many breaking changes moving to 6.0 anyhow? I can see
keeping the signatures the same in 5.x, but I, personally, am not as
concerned about keeping them the same moving to 6.0. I would certainly
try to make the migration as painless as possible.
I'm on the fence about whether empty-creation configuration should
apply per embedded vs embeddable.
I'm going to create a gist to summarize what we've discussed, and we
can discuss further there.
On Tue, Oct 17, 2017 at 8:04 AM, Steve Ebersole wrote:
> My one concern with doing this in 5.x is that we'd almost certainly change
> the signature of EmptyCompositeStrategy as we transition to 6. 6.0 has a
> real encapsulation of the embeddable in addition to the embedded. 5.x only
> really keeps the embedded (CompositeType).
>
> Anotger aspect that we have not decided is whether this empty-creation
> applies to the embeddable or the embedded. The checks would all be
> embeddable specific - the state initialization is part of the embeddable
> (the Class) so it would not change between embedded usages. We would have to
> decide if it makes sense to allow different empty-creation decisions for
> each embedded usage of that embeddable.
>
>
> On Tue, Oct 17, 2017, 9:42 AM Steve Ebersole wrote:
>>
>> On Tue, Oct 17, 2017 at 12:55 AM Gail Badner wrote:
>>>
>>> >
>>> > WRT to the condition check, you suggest to "compare a primitive value
>>> > in a
>>> > composite with the default for the primitive type (e.g, comparing an
>>> > int
>>> > value with 0 instead of null)". That's not the best condition check.
>>> > I was
>>> > pointing out that we already have better handling for this for ids and
>>> > versions and that we ought to follow that paradigm, mainly on startup
>>> > we
>>> > would instantiate an instance of said component and check the initial
>>> > values
>>> > for its attributes. E.g., consider:
>>> >
>>> > @Embeddable
>>> > public class MyEmbeddable {
>>> > private int someInt;
>>> > private int anotherInt = -1;
>>> > }
>>> >
>>> > Here, comparison of `#anotherInt` to 0 is not really correct. The
>>> > proper
>>> > comparison is to -1. Again, this is exactly what we do for ids and
>>> > versions.
>>> >
>>> > So because your (A) and (B) check the same condition I was pointing out
>>> > that
>>> > they actually are based on the incorrect condition check (0 versus -1),
>>> > so
>>> > we should correct that assumption. But even then we have and further
>>> > that
>>> > we should update the check and that we still have the different outcome
>>> > between (A) and (B) to decide between.
>>>
>>> I have already implemented something that does the check that you
>>> suggested, and I decided against it for a couple of reasons:
>>> 1) Any initialized values can be "real" values.
>>> 2) Suppose an instantiated embedded value includes something like:
>>> Date created = new Date();
>>> double random = Math.random();
>>> For 2), an embedded value would never be considered empty, and its
>>> values would always be persisted instead of null.
>>
>>
>> I'm sorry but I completely disagree here.
>>
>> The Date one is interesting but ultimately can be handled somewhat easily.
>> I'll discuss why/how below.
>>
>> And the "random" is completely a fabricated case I'm guessing. Can you
>> show me a user reporting that as a real use case? Regardless I think you'll
>> agree that this is very less-than-common use-case. And anyway, it too can
>> be mitigated by the same handling I hinted at with Date. Consider:
>>
>> @Embeddable
>> class MyEmbeddable {
>> Date created = new Date();
>> double random = Math.random();
>> }
>>
>> ...
>>
>> final MyEmbeddable e1 = new MyEmbeddable();
>> final MyEmbeddable e2 = new MyEmbeddable();
>> if ( theCompositeType.isEqual( e1, e2 ) ) {
>> // we can recognize and leverage initial values
>> ...
>> }
>> else {
>> // we can't (your date/random examples)
>> //- this branch requires that either:
>> //- a strategy is supplied, or
>> //- creation of empty is disabled
>> ...
>> }
>>
>> These kind of decisions are all about covering common cases and then
>> allowing config or SPI to cover the less-common ones. It boils down to what
>> you think is more common:
>>
>> double someNumber = -1;
>>
>> or:
>>
>> double someNumber = Math.random();
>>
>> Clearly initializing to a set (magic) number is far more common.
>>
>> Also, I think you have to ask yourself about the likelihood that such a
>> component with its values initialized as such will ever be empty. At least
>> for those specific values, I think it is HIGHLY unlikely.
>>
>>
>>>
>>> I think that it should be up to the `EmptyCompositeStrategy` to decide
>>> if a primitive that has the default value, or something like `created`
>>> or `random` should be ignored when determining if an embedded value is
>>> empty.
>>
>>
>> While I do not disagree that EmptyCompositeStrategy is the most flexible
>> solution (
