Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Question: How would you implement a many to many relationship? #890

Closed
w00519772 opened this issue Dec 24, 2018 · 11 comments
Closed

Question: How would you implement a many to many relationship? #890

w00519772 opened this issue Dec 24, 2018 · 11 comments
Labels

Comments

@w00519772
Copy link

w00519772 commented Dec 24, 2018

Thanks again once again for developing this app.

My question follows on from this one: #869.

I don't believe eshopcontainers contains any many to many relationships - if I am wrong then do let me know,

Say I have a many to many relationship i.e. a Book has many Categories and a Category has many Books. With EF I would have two classes i.e. Book and Category. Then Book would have a collection of Categories.

However, with EF Core; it appears you must have a join entity i.e. Book Category as described here: dotnet/efcore#1368. Therefore you have three classes i.e. Book; Category and BookCategory. A Book has a collection of BookCategories and a Category has a collection of BookCategories. I was recently reading this: https://blog.oneunicorn.com/2017/09/25/many-to-many-relationships-in-ef-core-2-0-part-4-a-more-general-abstraction/

I am asking this question strictly from a "best practice" perspective. Would you:

  1. Follow the advice in the hyperlink and create a Façade to hide the join entity.
  2. Use a join entity and forget about it
  3. Do something else

Please assume that the join table in the database contains foreign key vales only.

It appears the questions I have asked so far are logged as issues. Please could this be changed to an question? If someone could explain how to create questions then I would be grateful - I have spent some time Googling this and have not found an answer.

@zeehjr
Copy link

zeehjr commented Dec 26, 2018

No, you will need a join model only if you need Category with many Books, and Books with many Categories.

@w00519772
Copy link
Author

w00519772 commented Dec 27, 2018

      No, you will need a join model only if you need Category with many Books, and Books with many Categories.

I have edited my question based on your response. It now says: "Say I have a many to many relationship i.e. a Book has many Categories and a Category has many Books". Originally there was a typo and it said: "Say I have a many to many relationship i.e. a Book has many Categories and a Category has many Authors".

Which option would you choose i.e. 1, 2 or 3?

@jacobbutton
Copy link

I attempted to use the facade method in your linked article. The main issue I found with it is if you're utilizing AsQueryable() to drill down in your queries based on multiple filters, you won't be able to filter via the facaded child entities without first enumerating the query, which defeats the whole purpose of AsQueryable.

If you aren't worried about that then go for it, but I found it's more trouble than it's worth.

@gojanpaolo
Copy link

I've been doing 2 (Use a join entity and forget about it) and found no problems doing so. If only a small number of developers (<=5) will use your model classes, then just use the join entity.

I tried 1 (create a Façade to hide the join entity) and I agree that it's more trouble than it's worth.

@w00519772
Copy link
Author

w00519772 commented Dec 27, 2018

@gojanpaolo, what if there is more than 5 developers? Please assume the team size is 12. Is there another approach?

@w00519772
Copy link
Author

@jacobbutton, I believe using AsQueryable is a bad idea because it gives people control to do whatever they want i.e. outside your control.

What would you do if you were worried about this i.e. you want to use IQuerable? Is there another approach?

@gojanpaolo
Copy link

@w00519772 12 doesn't seem to be that large also but it would be a good idea to consult with them if they're ok with using the join entity. Personally, i'd still push for 2. It's a lot simpler and once EF Core has a many-to-many built-in, we can then mark the join entity obsolete if it's unwanted.

@jacobbutton
Copy link

@w00519772 - I'd imagine nobody but you, or your internal developers, should have access to what AsQueryable does. I agree, you shouldn't be publicly exposing it. But there are scenarios where it's a valuable tool for you to use (such as conditional filtering - better to have SQL server do all the filtering on enumeration than it is to do the filtering on a large query in application memory) before ultimately enumerating and returning the list to your end user.

Take a scenario like this.

public async List<Activity> GetList(ActivityFilter filter)
{
    var query = context.Activities.AsQueryable();

    if (filter.TypeId.HasValue)
    {
        query = query.Where(activity => activity.TypeId == filter.TypeId.Value);
    }
    if (filter.IsCompleted.HasValue)
    {
        query = query.Where(activity => activity.IsCompleted == filter.IsCompleted.Value);
    }
    if (filter.AssignedId.HasValue)
    {
        query = query.Where(activity => activity.Assigned.Any(assigned => assigned.Id == filter.AssignedId.Value));
    }
           
    return query.ToListAsync();
}

There's advantages in doing this in that that database isn't actually queried until the .ToListAsync() function is called, and it's queried and sorted based on all your predicates at once in SQL server which is more optimized to handle it. Now assuming Assigned is a many to many relationship, you aren't going to be able to filter on that if you're using the Facade method.

@w00519772
Copy link
Author

@w00519772 - I'd imagine nobody but you, or your internal developers, should have access to what AsQueryable does. I agree, you shouldn't be publicly exposing it. But there are scenarios where it's a valuable tool for you to use (such as conditional filtering - better to have SQL server do all the filtering on enumeration than it is to do the filtering on a large query in application memory) before ultimately enumerating and returning the list to your end user.

Take a scenario like this.

public async List<Activity> GetList(ActivityFilter filter)
{
    var query = context.Activities.AsQueryable();

    if (filter.TypeId.HasValue)
    {
        query = query.Where(activity => activity.TypeId == filter.TypeId.Value);
    }
    if (filter.IsCompleted.HasValue)
    {
        query = query.Where(activity => activity.IsCompleted == filter.IsCompleted.Value);
    }
    if (filter.AssignedId.HasValue)
    {
        query = query.Where(activity => activity.Assigned.Any(assigned => assigned.Id == filter.AssignedId.Value));
    }
           
    return query.ToListAsync();
}

There's advantages in doing this in that that database isn't actually queried until the .ToListAsync() function is called, and it's queried and sorted based on all your predicates at once in SQL server which is more optimized to handle it. Now assuming Assigned is a many to many relationship, you aren't going to be able to filter on that if you're using the Facade method.

Thanks. I see what you mean. With your example you are not exposing an IQueryable - it is just an implementation detail.

Are you saying that option 2 is "best"? Thanks,

@mvelosop
Copy link
Collaborator

mvelosop commented Jan 8, 2019

Hi @w00519772, I agree that option 2 is the best, at least for now.

As for IQueryables, which are better kept hidden as @jacobbutton comments, I could also recommend you take a look at the Query Specification Pattern

Hope this helps too!

@mvelosop
Copy link
Collaborator

mvelosop commented Jan 8, 2019

Closing this issue now as it seems to be solved, feel free to comment though, will reopen if needed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants