ardalis / CleanArchitecture

Clean Architecture Solution Template: A starting point for Clean Architecture with ASP.NET Core
MIT License
16.25k stars 2.8k forks source link

Question: Application Core Services code explosion #57

Closed politigf closed 5 years ago

politigf commented 5 years ago

I liked the layout and ideas in CleanArchitecture (thank you for your work!) so I've implemented it in our new application.

One thing I'm noticing as development continues, is that our services in the ApplicationCore project that utilize the repositories and specifications, are causing large amount of code to be added as I add various specifications to support our requirements (Typically specifications with differing breakdowns of what navigable entities are included rather than one huge 'bring back the world' call). I understand the reasoning behind the separation and not using the context directly at that layer. However, it is getting to the point that it is making me re-think if it's the best setup for us. This led me to find this article and wonder if I should change to utilizing services in the infrastructure project that access the context directly.

I'd appreciate peoples thoughts on this - and/or if am I misunderstanding something.

ardalis commented 5 years ago

This is a great question that has a few areas to discuss that require me to either make assumptions or get more information. I'll go with the latter for now and ask whether you're using domain events, or if you're using application services instead for any operations that work on your entities but need dependencies to perform additional work?

Usually when I see "application core services explosion" the reason is that too much logic is going into services instead of into entities where it belongs, and usually the reason is that the code has dependencies and the dependencies can't be injected into entities so the natural thing to do is move it into services. Is that the case for your app?

Also sorry for delayed response - just got back from vacation.

politigf commented 5 years ago

We are making use of domain events. Limited use at this point as we are early in development, but we have them set up and working.

[issue 1] We do have some instances of having too much logic in the ApplicationCore services. So far, they seem to be due to populating various collection properties on an entity dynamically based on table data. Our exact entity types can be confusing, so a similar example requirement would be:

Whenever a Customer entity is created it's addresses collection should be populated based on the rows in the CustomerAddressDefaults table. The CustomerAddressDefaults table can change over time, but only newly created Customers going forward would get those new defaults.

We weren't sure how to accomplish that using DDD and keeping that logic in the entities, so it exists in the AppCore services.

[issue 2] The specifications we've been creating is what prompted me to ask the question initially. Going back to the simplified example above, say our Customer entity has multiple collections, each of which can have nested collections, and so on. In our various service methods, each method typically only has need for a subset of all of those collections. This causes us to write a unique specification class (e.g. CustomerWithAddresses, CustomerWithOrders, CustomerWith.., ... ... .. )for each variation of the customer entity with related data, rather than writing a single specification that includes every possible piece of data.

We are in the early phase of development for this project, and the number of specifications we have created and the speed-bump it causes when adding a service method as opposed to just utilizing the context.include/theninclude directly (if we moved the services to the Infrastructure project) has us rethinking our approach.

No apology needed, vacation is good, and we appreciate your work - thank you!

ardalis commented 5 years ago

[issue 1] Your entities should typically assume they're fully populated. Controlling that is typically the repository or specification's job. yes, this means you might have specs for different amounts of population as you describe (CustomerWithOrders, etc.). If this becomes a headache you can always pass in variables to the specification constructor that inform it how much of the object tree to include (e.g. new CustomerSpec(withOrders=true)).

[issue 2] I guess I answered this in the above.

If you move specifications to infrastructure so they can directly use dbContext you won't be able to use them from Core, and the only way you'll be able to use them from Web is if you take a direct dependency on Infrastructure (which you probably already have, but only to allow Startup to wire things up, not so various controllers and such can reference types there). So, I would avoid this approach.

politigf commented 5 years ago

Using the specification constructors option you mention in issue 1 is a good idea and will help with having numerous classes with only a difference in a single or few AddInclude lines.

For issue 2 - we weren't looking to move the specifications to the infrastructure project, we were looking at moving away from specification/repository use entirely, and having all services in the infrastructure project. The services then can use the context directly and we would get increased development fluidity, with the understanding that we would be tightly coupled to EF Core. Are there any other drawbacks? Before I make that jump to this, I'm trying to make sure I have a full understanding of what we would be giving up if we go that way.

ardalis commented 5 years ago

If the services don't have any business logic, then you're fine to do what you describe. Of course, if they don't have any business logic then they're just data access logic which pretty much makes them repository implementations (a repository is just a service whose only responsibility is data access). You can usually recognize business logic in the form of conditional statements.

If the services do have business logic, then the downside to putting them in infrastructure and tightly coupling them to EF is testability and flexibility. You'll probably only be able to test them using integration tests, but you can probably still use EF Core's InMemory provider so it shouldn't be that bad. And you won't be able to easily swap out data providers or use patterns like Decorator to implement caching between your business logic and your data access logic (see: https://ardalis.com/building-a-cachedrepository-in-aspnet-core).

Other than that it should work for you.

politigf commented 5 years ago

Thank you so much for your help - that is exactly the type of info we were looking for as we try to make a decision on how best to proceed.