I believe I might have a fix. At least the samples are running now.
Niclas will You review?
I still need to create a few unittests instead of testing indirectly through
the samples. And i would like to explore a little bit regarding values and
UnitOfWork's.
There is plenty of room for optimization though.
The current impl allows the user to
ValueBuilder<X> b = vbf.newValueBuilder(X.class);
X p1 = b.prototype();
p1.att1().set("value1");
X inst1 = b.newInstance();
p1.att1().set("value2");
X inst2 = b.newInstance();
eg the reference to the prototype is still valid after creating a value
(inst1). The downside is that we have to clone the prototype always. If we
instead require the user to fetch a new reference to the prototype:
ValueBuilder<X> b = vbf.newValueBuilder(X.class);
X p1 = b.prototype();
p1.att1().set("value1");
X inst1 = b.newInstance(); // p1 is now immutable
X p2 = b.prototype();
p2.att1().set("value2");
X inst2 = b.newInstance();
it would be possible to avoid cloning in the common cases - it should be pretty
straightforward to let newInstance() return the prototype (and making it
immutable), and then clone (and make mutable) on the next invocation of
b.prototype().
Would that be acceptable?
The cloning itself could probably be optimized too - but I think that would be
of minor importance, if the change above is implemented.
Note that builders created by Module#newValueBuilderWithPrototype and
Module#newValueBuilderWithState are still use-and-throw-away: these builders
are currently unable to create more than a single value.
/Kent
Den 24/06/2012 kl. 17.57 skrev Niclas Hedhman:
> Roughly speaking; The "module" of the Value is not the correct
> visibility, since the currentUnitOfWork is from the ContextLayer, and
> calling currentUnitOfWork() wraps a visibility of the module of the
> Value, i.e. Domain Layer.
>
> This is primarily related to the 'clumsy' (;-) implementation, where
> you probably didn't realize the consequences... I will try to sort it
> out, IF we decide that PROTOTYPING should be allowed.
>
> Cheers
> Niclas
>
> On Sun, Jun 24, 2012 at 8:43 PM, Kent Sølvsten <[email protected]> wrote:
>> Hi Niclas, great to have You back.
>>
>> As You can see, things have been pretty slow - I myself has been on vacation
>> in the Caribean (sweeeeeeet) - and then ill ( not so sweet).
>>
>> Allow me to shed some light on this, which may or may not be useful.
>>
>> The original (visible) problem with ValueBuild'ing was that the same
>> ValueBuilder could not be reused for creating several values.
>> The cause was that the build() was simply returning the prototype object,
>> requiring us to lock further modifications to ensure immutability.
>> We create ValueBuilders in 3 ways:
>>
>> 1) Module#newValueBuilder( Class<T> mixinType ) - the only one commonly
>> used by application code
>> 2) Module#newValueBuilderWithPrototype( T prototype ) - seems to be
>> primarily used when serializing/deserializing
>> 3) Module#newValueBuilderWithState(.....) - used internally from 2).
>>
>> My attempt was to ignore cases 2) and 3), since I was unable to see a great
>> usecase for creating several values from these valuebuilder.
>> The attempted solution was to create several implementation classes for
>> ValuBuilders. The implementation of case 1) uses
>> Module#newValueBuilderWithPrototype to clone the prototype object.
>> I believed that this approach should be sound, although probably not the
>> most efficient.
>>
>> My thinking was that If any state is lost, then it would be a bug inside the
>> ValueBuilderWithPrototype, which should be fixed in its own right. The
>> serialization/deserialization triggered is an implementation detail inside
>> the ValueBuilderWithPrototype.
>>
>> Can You explain in more detail what is going wrong in the test?
>>
>> /Kent
>>
>>
>>
>> Den 24/06/2012 kl. 11.18 skrev Niclas Hedhman:
>>
>>> More info on this; So, the reason for the failure in the test is due
>>> to the serialize/deserialize of Values that happens on creation. This
>>> is not the way it should be handled, and Marc's original code may not
>>> be incorrect.
>>>
>>> It boils back to that cloning is hard, and Kent took the easiest
>>> approach, albeit an incorrect one.
>>>
>>> Now, the solution can be;
>>> 1. Record the actions on the ValueBuilder instead of settings of
>>> state, then the replay of those set actions are used to build the
>>> state on a new ValueInstance each time. This is a straight-forward
>>> approach with a wrapper around the builder.
>>>
>>> 2. A more interesting case would be to introduce a delta inside the
>>> ValueInstance. So after the first ValueInstance is built, any changes
>>> are recorded as deltas and the 'primary' state (the first value
>>> instance) is shared across all value instances that was built using
>>> the same builder. This could save quite a bit of memory if values has
>>> many properties and only small variants.
>>>
>>> 3. Prototyping support is removed.
>>>
>>> I am leaning towards 3. myself at the moment, to reduce scope and get
>>> nearer 2.0
>>>
>>>
>>> -- Niclas
>>>
>>> On Sun, Jun 24, 2012 at 3:19 PM, Niclas Hedhman <[email protected]> wrote:
>>>> FYI,
>>>> IF I also add LocationEntity and VoyageEntity to the Domain Layer,
>>>> with Visibility.module, the tests pass. But not sure if that is
>>>> correct design.
>>>>
>>>> On Sun, Jun 24, 2012 at 3:16 PM, Niclas Hedhman <[email protected]> wrote:
>>>>> I took a look at the test failure in DCI sample B.
>>>>>
>>>>> The CarrierMovement in layer "DomainLayer" VALUE has a Property to a
>>>>> Location, but the only Location defined is a LocationEntity sitting in
>>>>> a higher layer ("ContextLayer").
>>>>>
>>>>> Marc!!!
>>>>> How is this intended to work?
>>>>> CarrierMovement is not allowed to access higher layers like that. It
>>>>> might have worked previously due to the creation of the UnitOfWork
>>>>> happening in the ContextLayer, but since the ValueBuilder was changed
>>>>> to support prototyping, this is surfacing as a probable bug in the
>>>>> sample.
>>>>>
>>>>>
>>>>> Cheers
>>>>> --
>>>>> Niclas Hedhman, Software Developer
>>>>> http://www.qi4j.org - New Energy for Java
>>>>>
>>>>> I live here; http://tinyurl.com/3xugrbk
>>>>> I work here; http://tinyurl.com/6a2pl4j
>>>>> I relax here; http://tinyurl.com/2cgsug
>>>>
>>>>
>>>>
>>>> --
>>>> Niclas Hedhman, Software Developer
>>>> http://www.qi4j.org - New Energy for Java
>>>>
>>>> I live here; http://tinyurl.com/3xugrbk
>>>> I work here; http://tinyurl.com/6a2pl4j
>>>> I relax here; http://tinyurl.com/2cgsug
>>>
>>>
>>>
>>> --
>>> Niclas Hedhman, Software Developer
>>> http://www.qi4j.org - New Energy for Java
>>>
>>> I live here; http://tinyurl.com/3xugrbk
>>> I work here; http://tinyurl.com/6a2pl4j
>>> I relax here; http://tinyurl.com/2cgsug
>>>
>>> _______________________________________________
>>> qi4j-dev mailing list
>>> [email protected]
>>> http://lists.ops4j.org/mailman/listinfo/qi4j-dev
>>>
>>
>>
>> _______________________________________________
>> qi4j-dev mailing list
>> [email protected]
>> http://lists.ops4j.org/mailman/listinfo/qi4j-dev
>
>
>
> --
> Niclas Hedhman, Software Developer
> http://www.qi4j.org - New Energy for Java
>
> I live here; http://tinyurl.com/3xugrbk
> I work here; http://tinyurl.com/6a2pl4j
> I relax here; http://tinyurl.com/2cgsug
>
> _______________________________________________
> qi4j-dev mailing list
> [email protected]
> http://lists.ops4j.org/mailman/listinfo/qi4j-dev
>
_______________________________________________
qi4j-dev mailing list
[email protected]
http://lists.ops4j.org/mailman/listinfo/qi4j-dev
