256101 (5) [Avatar] Offline
#1
Hi Jon,
I'm really enjoying the book. Here is some feedback, ordered by pageNumber. Some points probably more important than others, but I mention them all for completeness. Hope you find this feedback usefull.


1/ p44 Strange sentence : 2.6.1 With eager loading I could to load ALL the books and then, in memory, sort or filter the ...
2/ p69 Strange sentence : I use these terms in throughout the book, where applicable
3/ p70 Note: I could have provided a backward link to the book, but I didn't. I explain why in chapter 7
I don't understand what you mean by backward link, in fact PriceOffer has a property BookId, isn't that a backlink ?
4/ p77 Naming : public Review GetBlankReview(int id) rename id in bookId
5/ p98 Listing 4.5 => bullet circle "1" :
What is the need for the extra properties Errors and HasErrors ?
They are already present in the actionclass passed in by the caller (via the IBizAction interface ), and so available everywhere
(same issue in RunnerWriteDbWithValidation on p 106 and others )
I would remove them, less is more smilie

6/ p99 In my opinion PlaceOrderService:smilielaceOrder should have a PlaceOrderInputMessage argument and return an
PlaceOrderOutputMessage.

I find it very odd to see Cookie related stuff in a serviceLayer containing Business logic.
Aren't those BusinessLogic classes supposed to be calleble /re-useable from any kind of application (even a winforms app )
The cookie stuff should be moved to the MVC controller code, where the ServiceLayer is called, based on the return values in the PlaceOrderOutputMessage class.


7/ p104 Listing 4.6
[Range(1,5, ErrorMessage = "This book is over the limit of 5 books."

=> Shouln't this be "This order is over the limit of

8/ p105 Having the added validation rule code to my LineItem entity class, I need to add a validation
stage to EF Core’s SaveChanges method, which I have called SaveChangesWithChecking.

=> Why not SaveChangesWithValidation, to avoid different wordings Validation / Checking

Regards,
StephDep




Jon P Smith (9) [Avatar] Offline
#2
Hi StephDep,

Thanks for taking the time to feedback on the book content. The deadline for the next MEAP update has passed, but I have I have gone through your comments and noted the changes you suggested when the next update comes up. I especially like your suggestion to change SaveChangeWithChecking -> SaveChangesWithValidation, which I will do when I have a major update of the code.

I few things you mentioned are worth commenting on:

5/ p98 Listing 4.5 => bullet circle "1" :
What is the need for the extra properties Errors and HasErrors ?

Because the BizLogic is almost at the bottom of the layered architecture then the Errors and HasErrors properties have to be define at the BizLogic layer. The upper layers define their own Errors and HasErrors properties to match the BizLogic's interface.

I find it very odd to see Cookie related stuff in a serviceLayer containing Business logic.

This is an important point for me, but one I obviously haven't explained enough. For me the ServiceLayer it the interface between all the layers below it, and the presentation layer. Therefore the ServiceLayer known about both worlds: the lower layers under it and the ASP.NET Core layer above. The ServiceLayer takes on as much of the adapting as it can. Also, the CookieService was originally in the ASP.NET Core part, but it was hard to test, as it was linked directly into the HttpContext. I therefore moved it into the ServiceLayer and used IRequest/IResponseCookies interfaces, which made it much easier to test.

I hope that is helpful feedback for you, as your feedback has been useful for me. A new MEAP should be out soon with two new chapter on configuring EF Core.

Kind regards

Jon P Smith

256101 (5) [Avatar] Offline
#3
5/ p98 Listing 4.5 => bullet circle "1" :
What is the need for the extra properties Errors and HasErrors ?


Because the BizLogic is almost at the bottom of the layered architecture then the Errors and HasErrors properties have to be define at the BizLogic layer. The upper layers define their own Errors and HasErrors properties to match the BizLogic's interface.


Still I don't see the use of these properties in the RunnerWriteDb class. To me they only make sense in the PlaceOrderService class. (see attached files )

Jon P Smith (9) [Avatar] Offline
#4
Hi StephDep,

You are right that the PlaceOrderService could be made simpler, but I used that form of call because I knew I was going to introduce the the two other BizRunners, RunnerWriteDbWithValidation and RunnerTransact2WriteDb, whichI cannot pass back the PlaceOrderAction errors on its own.

On page 107 of the book I said:
The nice thing about this new variant of the BizRunner pattern is that it has exactly the same
interface to the original, non-validating BizRunner. That means I can substitute the
RunnerWriteDbWithValidation<TIn, TOut> for the original BizRunner without needing to
change the business logic or the way that the calling method executes the BizRunner.

I hope that makes that clearer. I try to standardize how I call things so that changing between different runners is quick and easy.

Jon P Smith
Jon P Smith (9) [Avatar] Offline
#5
Hi StephDep,

I have just done a tidy up of the first five chapters and included many of your corrections. I don't know when the next MEAP release will be out, but you should see these change appear soon.

Thanks for your feedback.
256101 (5) [Avatar] Offline
#6
Thanks Jon, great to hear. I'm looking forward to read the new chapters.