Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Microsoft.EntityFrameworkCore dependency in Application layer #439

Closed
vccampbell opened this issue Sep 2, 2021 · 6 comments
Closed

Microsoft.EntityFrameworkCore dependency in Application layer #439

vccampbell opened this issue Sep 2, 2021 · 6 comments

Comments

@vccampbell
Copy link

Hi. I was curious if we should have a dependency on the database ORM within the Application layer? Specifically we're referencing Microsoft.EntityFrameworkCore in Application > TodoLists > Queries > GetTodos > GetTodosQuery.cs. Seems like this should be abstracted away from the business logic, maybe opting for Repository pattern to inject specific functionality.

private readonly ITodoRepository todosRepository;
public GetTodosQueryHandler(ITodosRepository todosRepository)
{
this.todosRepository = todosRepository;
}
public async Task<TodosVm> Handle(GetTodosQuery request, CancellationToken cancellationToken)
{
// ...
Lists = await todosRepository.GetTodoListsAsync(cancellationToken);
}

Appreciate any advice/guidance.

@ronnieholm
Copy link

That's quite deliberate. I believe Jason covers why in his talk.

That dependency is there because it's the database agnostic part of Entity Framework. The MS SQL server dependency is in Infrastructure.

Sure, you can abstract it away behind a repository, but then you have a repository wrapping a repository (DbSet<T> is a repository). It would result in boilerplate code that in most cases would serve no real purpose.

@vccampbell
Copy link
Author

Hi @ronnieholm, thank you for the reply and it makes perfect sense. If my application had a mix of EF and Dapper, would the recommended implementation be to abstract the Dapper bits behind a repository?

Thanks again.

@sk0va
Copy link

sk0va commented Oct 8, 2021

It definitely breaks Dependency Rule from the original The Clean Architecture article by Robert C. Martin!
The overriding rule that makes this architecture work is The Dependency Rule... (!)
...This rule says that source code dependencies can only point inwards. Nothing in an inner circle can know anything at all about something in an outer circle. In particular, the name of something declared in an outer circle must not be mentioned by the code in the an inner circle. That includes, functions, classes. variables, or any other named software entity.
Also it's a bad idea to use DbSet<> directly from business logic (i.e. DeleteTodoListCommand), because it introduce vendor lock on choosen infrastructure framework (EF Core in this case). What if we want to switch from EF.Core to other ORM like Dapper of NHibernate or whatever? What if we want to even switch storage type from SQL (like MS SQL Server) to NoSQL (like Mongo)? You can imagine what efforts such changes will require. That is why Dependency Rule is so important - it allow to make changes in outer levels without affecting inner levels.
In my opinion, in this solution should be used something like Repository pattern, to abstract persistence logic and separate it from business logic.

@karimkod
Copy link

karimkod commented Oct 8, 2021

@seoriy I totally agree with you on that one however, this is a choice for the architect to make and Jason has backed his choice in all his talks. Talking about the clean architecture book, Uncle bob also made an exception by saying

That’s normal—but it should still be a decision. You must understand that
when you marry a framework to your application, you will be stuck with that
framework for the rest of the life cycle of that application. For better or for
worse, in sickness and in health, for richer, for poorer, forsaking all others,
you will be using that framework. This is not a commitment to be entered
into lightly

Jason said that's he is ok with coupling with EntityFramework since he used it for a very very long time.

However, you can make the change easily in the template. It's not referenced everywhere, except for the DbSet dependency.

@sk0va
Copy link

sk0va commented Oct 8, 2021

@karimkod

Uncle bob also made an exception by saying

I think he was talking about a bit other cases. I.e. if you use .NET Framework to write application, it's obvious, that you use BCL even to describe domain logic, so you may use System.Collections.Generic.List<> for domain logic, without treating it as a violation of The Dependency Rule. Same for IDisposable, LINQ, etc.
And right after that paragraph he saying:

CONCLUSION
When faced with a framework, try not to marry it right away. See if there aren’t
ways to date it for a while before you take the plunge. Keep the framework behind
an architectural boundary if at all possible, for as long as possible
. Perhaps you can
find a way to get the milk without buying the cow.

Also

Jason said that's he is ok with coupling with EntityFramework since he used it for a very very long time.

It's only his personal choise. Someone else may want to use this template, but replacing EF Core with Dapper, for example...

@karimkod
Copy link

karimkod commented Oct 8, 2021

@seoriy Yes, the conclusion was for the chapter, the section above it was for setting the exception.

And yes, everything is relative. As I said before, you can change this template to match your own definition.

And again, this is not an absolute solution/template everything is flexible, you can build on it, change it, make it perfect for your own decisions.

Repository owner locked and limited conversation to collaborators Nov 10, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants