eShopOnWeb has a couple of its service classes (BasketService and OrderService) implement corresponding interfaces (IBasketService and IOrderService). These interfaces have no other implementations (even in test projects) so the value of including them in the architecture is highly questionable.
IBasketService and its references are shown below… click on the image for a larger view in a new tab. Single implementation interfaces in my opinion are just pointless. The whole point of using interfaces is that we can swap out multiple implementations without our calling code having to care about which actual concrete class is doing the end work.
When we have only one implementation of an interface, we are paying the cost of having the interface without getting any benefit. It’s easy to extract an interface when it’s needed so there’s no need IMHO to create interfaces ahead of time ‘just in case’.. and always remember YAGNI.
Using interfaces to aid testing
Testing is a special case whereby we might only have one implementation of an interface in our app itself but we have another implementation in our test project to facilitate plugging in mocked or stub test versions of our classes. In this case we still have multiple implementations… whether it’s a good idea to add a load of interfaces to our apps just for testing is debatable and is a question for a different day but we definitely are not in a single implementation scenario.
Using interfaces for DI
We don’t need interfaces to use dependency injection, this works…
Does it make sense to ever have multiple implementations of service classes?
Of course eShopOnWeb is just a reference architecture, it’s not a completed app, if it were perhaps there would be more implementations of IBasketService and IOrderService… BUT still the problem I see with this is that our services classes (and below) are basically our apps. The only thing we usually have above our service layer might be a thin HTTP or console app layer.
Our service classes are the main way into all the use cases and functionality of our app and contains all our business logic and usually already have their dependencies passed in. This means we don’t need multiple implementations for testing.
What about having multiple implementations in our app itself? Well are we really going to have multiple ways to implement our core use cases? For example below in the AddItemToBasket method we..
Check for existing basket
If there is none we create one
Then add the item to the basket
Then persist it to our datastore
The four steps needed to add an item to basket won’t change, if they do it’s a different use case and will be implemented by a different method.
IMHO the inclusion of the IBasketService and IOrderService interfaces just adds bloat and unnecessary complexity to the architecture.
eShopOnWeb uses the repository pattern over Entity Framework ((I’m not getting into this debate again 😉). When we use this pattern we can sometimes see a proliferation of similar methods such as GetBasketByBuyerId, GetBasketByBasketId etc.
eShopOnWeb uses the specification pattern to encapsulate queries and their details. This means we don’t have a proliferation of repository methods but rather just a few (eg. GetBySpecAsync) which take a ‘specification’ as input. The queries however have really just been moved to another location and we’ll likely still have a similar amount of specifications as repository methods.
For example… below we can see that instead of having two repository methods with a query each for GetBasketByBasketId and GetBasketByBuyerId scenario eShopOnWeb has a BasketWithItemsSpecification class which has two queries via two different constructors.
Click on the image for a larger view in a new tab…
What does everyone think? … for me this is kind of like taking an abstraction (the repository pattern) and then splitting it in two. It’s makes the architecture harder to navigate and understand for little if any benefit.
eShopOnWeb has a couple of uses of AutoMapper just so developers can see how to wire it up but in general to me AutoMapper is really overused.
I’ve seen developers take onboard this dependency for the sake of saving 15 lines of code… but… it has a real cost which many developers don’t appreciate.
It’s another ‘thing’ in the architecture, its another thing that has to be documented, thought about, talked about, it’s another thing you have to give any new developer time to get used to, its another thing to debug, another thing which can break, another thing that can have security vulnerabilities, another thing to upgrade, another thing that can stop being maintained etc..
I’ve used it on a couple of projects and it’s always cost more time than it has saved. It’s probably the best at what it does but even it’s author Jimmy Bogard knows its used in ways it ways he never intended for it to be used…
IMHO left->right assignment is amongst the easiest code any developer is ever going to write so I avoid downloading AutoMapper as simply put I don’t think it’s needed.
If you are using it, from my own experience I’ve found it is the custom mappers that cost most config and debug time so I’d recommend just using the default map capabilities and then using plain left-right assignment for anything that doesn’t map out of the box.. eg..
Aggregate roots are a central concept in Domain Driven Design. The idea is pretty simple and refers to how certain entities or objects should only be manipulated (and retrieved) in the context of their root or parent (or ancestor etc.) entity.
If we are able to centralize the point at which any objects in an aggregate graph are edited to a single object (the aggregate root) we can enforce rules and validations to ensure the whole graph remains valid.
In eShopOnWeb the repository pattern helps enforce non manipulation of anything but aggregate root classes. We can see below in the order aggregate that only the Order class implements IAggregateRoot and thus is the only class from this aggregate allowed to create a strongly typed repository to deal with persistence.
Click on the image for a larger view in a new window…
Of course we should note there is nothing stopping a developer from having OrderItem (for example) implement the IAggregateRoot interface. In this case they could create a repository for the OrderItem class and therefore bypass any logic contained in the root… but… there’s only so much we can do on a code level to guide developers.
IMHO due to the clear folder structure (eg. BasketAggregate, BuyerAggregate and OrderAggregate) as well as the use the IAggregateRoot marker interface eShopOnWeb does a really good job of letting developers know certain objects shouldn’t be retrieved and edited directly.
To improve performance eShopOnWeb uses IMemoryCache to implement caching to prevent queries being sent to the Database for data that doesn’t change at all or often.
In the case of the GetCatalogItems method below, we can see it will only call the non cached version of GetCatalogItems if the catalog items are not already cached. Each time we hit the cache we ‘save’ the three queries below… Depending on the amount of users and the exact queries used, implementing caching can make a big difference to the performance of an app but don’t implement it if you don’t need it.
IMemoryCache in a distributed environment?
eShopOnWeb uses IMemoryCache which is a local in memory cache, does this mean we can’t use IMemoryCache in a multi-host environment? Not necessarily, it will depend on what kind of data we are actually caching.
If for example we are caching static items which rarely change for display in dropdowns such as Brands or Types as shown above using a local cache may be OK. The only downside is that the first request for an item on a new host will hit theDB but this is a small price to pay for a simpler architecture.
If on the other hand we are caching data which might change, for example a product description the problem of invalidating that products entries in all local in memory caches will likely make using a centralized cache like Redis more appropriate.
Do we need both CachedCatalogViewModelService and CatalogViewModelService?
eShopOnWeb has two implementations of ICatalogViewModelService, CachedCatalogViewModelService which implements caching and is basically a wrapper (or decorator) around CatalogViewModelService which doesn’t implement caching. One advantage this gives us is that we can easily turn off caching for this service by swapping out the implementations in our DI configuration..
Of course an alternative is to have no interfaces and a single catalog ViewModel service class which takes in an instance IMemoryCache and has the caching code embedded. There is no right or wrong answer, I’m just thinking out loud… what would you do?