Hi Rickard!
Brilliant!!!! Wonderfully elegant.
Thank you for taking the time to look at my attempts and come up with your
improved version! I agree with you that the Qi4j-DCI setup is now clean, simple
and intuitive at the same time.
When we get @Concerns on POJOs (before christmas?), wouldn't we be able to
improve the context stacking intuitiveness with something as simple like this?:
@Query
public Integer availableFunds()
{
return sourceAccount.availableFunds();
}
@Command
public void transfer( final Integer amount ) throws IllegalArgumentException
{
sourceAccount.transfer( amount );
}
That would complete the extremely intuitive and powerful implementation I think.
Apart from that, I'm suggesting some minor cosmetic changes that relates to
bringing the code as close as possible to the Use case algorithm - at least the
one from the Lean Architecture book that i have pasted directly into the code.
I think that Jim has a point when he writes:
"[…] to the developer, these terms [source account, destination account]
provide strong implementation hints. DCI strives to capture such concepts
directly in the code." (p 201)
I'm taking this pretty literally, so I want to call the TransferMoneyContext
field "sourceAccount" instead of "source". Of course you and I know what
"source" means, but as a pedagodical example, I think it's important to convey
the importance of mapping the code naming *directly* to the Use case terms. It
can be a little more tedious to write as a programmer and seem unnecessary, but
I think that the code can easily slowly start to slide into reflecting the
programmer's rather than the end users mental model of the use case. (It's a
question I want to raise in the DCI forum).
I have saved your version without modifications as v9a, and the one with my
suggestions as v9b in qi4j_money_transfer_06.zip at http://bit.ly/bjjjMx. My
old v9a and v9b_draft are out. When I post the new implementation model in the
DCI forum, would it be okay for you if I save v9b as a new v9, and at the same
time refer to the "official" version at github
(https://github.com/Qi4j/qi4j-samples/tree/master/dci/)? Or do you want me to
leave v9a and v9b as they are now?
Comments to the cosmetic changes I have made in v9b:
Contexts
- I think that changing method name from Contexts.withContext(…) to
Contexts.enact(…) conveys more the action that is about to take place: the
enactment.
ContextInjectionProviderFactory.java
- Isn't it more correct to return a *Context*InjectionProvider (instead of a
*Role*InjectionProvider)? It's the Context that we inject. I know it's very
similar to InjectionContext, but it just feels confusing to see "Role" and
"Context" mixed up here when studying how it works.
TransferMoneyTest
- Expanded the logging messages to match the logging required by the Use case
algorithm.
- Fluent notation of the PayBills test, showing the 3 main steps needed to
enact the use case in one line:
Context instantiation -> Role binding -> Use case enactment
The Law of Demeter shouldn't be violated by this, since both the bind and
enactment methods are defined in the same Context that is instantiated. There
are no surprises, it's fast and it can't be more intuitive!
- The "re" prefix in rebind() seems unnecessary since in most cases where we
will bind the roles for the first time. "Re" indicates that it has happened
before. Like a setter method is not a resetter method, I suggest simply to call
it "bind". We could even have a case where only *some* roles where rebound, and
the Context role setup not entirely reset. "bind" can be used in all cases.
- Whether Data objects are instantiated inside a context is an implementation
detail left to the programmers choice (left all instantiation to the client as
you did). Maybe there would be cases where the Context is performing some logic
to decide which Data objects to instantiate where ids could be more suitable
(but that's another question for the DCI forum). That choice doesn't affect the
beauty of this implementation model.
TransferMoneyContext
- Shouldn't the first comment be "Roles are defined within the context, and
only the *Role Maps* know about them outside of this context"?
- Returned "this" from the bind method in order to be able to have a fluent
notation (as shown in second test)
- Added the Use Case algorithm here also, because I want to be able to compare
the code with the text (literally!). That's also why I added the
log(output-messages) as described in the use case algorithm.
PayBillsContext
- Comment in creditor loop changed to "// Re-bind Data in nested context and
execute enactment" (instantiation only happens once, before the loop)
If you have any comments, I'm all ears!
I'm ready to implement the Frontloading example with the new setup! :-)
Cheers,
Marc
Some inline comments:
On 16/11/2010, at 12.18, Rickard Öberg wrote:
> The main issue I could see is that PayBillsContext explicitly refers to
> TransferMoneyContext roles, when it should instead just use BalanceData.
Of course :)
> For some reason you changed the stack into a hashmap, and then it went wrong.
> Keep it as a stack, and then do injection from that instead.
Yes, erhm, thought I didn't need the stacking in order to do the lookup in the
InjectionProvider, but only some simple map (no popping and pushing).
> I have updated th qi4j-samples DCI sample to include the PayBillsContext, and
> have also added the @Context annotation for injection of context into role
> mixins. I also made the separation of entities into explicit rolemaps, as per
> your code. Please have a look at that.
It looks great!!
> AFAICT it's now pretty much perfect, apart from how the context stack is
> managed. We need Concerns on object methods to do that properly. But I have
> at least put the stack management in the context itself, which simplifies
> client code radically.
Yes! Great!
> Note that since I handle creditors as BalanceData the cast you had in your
> version is gone.
Perfect
> With the rebind() this just works, since it's the same context, just with a
> different binding. Tada!
Beautiful
> As above, I think the update I made in qi4j-samples just now represents the
> cleanest, simplest and most easily to understand version of how to do DCI in
> Qi4j.
Agree :-)
> Apart from the stack handling I can't think of anything I would want to
> improve. If you can find anything, please do let me know!!
Only cosmetic things as described above
_______________________________________________
qi4j-dev mailing list
[email protected]
http://lists.ops4j.org/mailman/listinfo/qi4j-dev
