The Author Online Book Forums are Moving

The Author Online Book Forums will soon redirect to Manning's liveBook and liveVideo. All book forum content will migrate to liveBook's discussion forum and all video forum content will migrate to liveVideo. Log in to liveBook or liveVideo with your Manning credentials to join the discussion!

Thank you for your engagement in the AoF over the years! We look forward to offering you a more enhanced forum experience.

RobertoI (29) [Avatar] Offline
#1
Hi Matthew,

I have enjoyed the book so far. I am currently reading the MEAP Version 2 and I have a few comments on Chapter 2.

1) Code to set the value of pointsPerDay in listings 2.4 and 2.7 is a bit more verbose than it should be: this could be simplified. For example:
int pointsPerDay = agreement.Vehicle.Size >= SizeLuxury ? 2 : 1;

2) In paragraph 2.2.4, when talking about introducing defensive programming techniques such as guard checks, you could probably mention the "code contracts" functionality offered by System.Diagnostics.Contract (such as Contract.Requires). I guess you could place this at the end of the paragraph or in a "side bar".

3) Regarding paragraph 2.2.7, the way you are suggesting using dependency injection to solve the aspect tangling problem is probably a bit unfair. Rather than injecting multiple "aspect services" into the LoyaltyAccrualService and the LoyaltyRedemptionService classes, and therefore cluttering the constructor of these classes with multiple aspect-related parameters, dependency injection could be implemented by creating separate "aspect decorators" of the underlying ILoyaltyAccrualService and ILoyaltyRedemptionService . Obviously this would create code-bloat because you would need to implement a specific "logging" class for each class that wants to be logged, for instance. But probably still worth mentioning...

4) The naming of the aspect classes you introduce progressively in paragraph 2.4. are not consistent with each other. You start with LoggingAspect. Then you introduce DefensiveProgramming (with no "Aspect" after it). Then TransactionManagement (still no "Aspect"), followed by ExceptionAspect. I think you should probably name all these attribute with Aspect (which I personally do not like), or none of them. Assuming you remove the suffix "Aspect" from all the aspect attributes, I still do not like the name "DefensiveProgramming": you could replace it with ParameterGuards, or even NullGuards, since the checks being performed are just null checks.

5) While you introduce the various aspects in paragraph 2.4 you never mention whether the sequencing of the aspect attributes on the Accrue and Redeem methods being untangled is important. For instance, should the ExceptionAspect attribute be place at the top or at the bottom of the attribute stack?

6) Small note on listing 2.27. Probably you can improve the logging by outputting the method name in this way:

Console.Out.WriteLine("{0} started : {1}", args.Method.Name, DateTime.Now);

Please note "started", which I have added to make this consistent with "complete" reported when the method completes.

I hope you find these comments useful.

Roberto
matthew.groves (46) [Avatar] Offline
#2
Re: Comments on Chapter 2
Roberto,

Thank you for reading the book and posting your comments here. I think you raise some good points, and I will take them into consideration.

Some specific responses:

My idea with (3) was that dependency injection alone (without interception) is an improvement, but still cluttered and tangled. Certainly the use of interceptor aspects with an IoC tool is a good idea, and that's what I recommend throughout the book. I will try to make this a little more clear.

With (5), the sequencing of aspects is not something that I wanted to spend time addressing in chapter 2. I believe there's a note or sidebar to that effect. The whole story is that C# attributes are applied in non-deterministic order, so you can't count on attribute order. I spend some time in chapters 8 and 9 (which I'm writing now) about PostSharp's mechanism for dealing with this (as well as Castle DynamicProxy).

Thanks,

Matt