2

I'm still learning .NET Core 2.1. I'm working on a Web API where I use EF Core. Currently I'm working on a many to many relationship between Users and Roles. I wanted to hide the pivot, but it ended out being a bit hacky i think, so I wanted to see what I could do to improve it.

I started out with something like this:

public class User
{
    public int Id { get; set; }
    public string UserName { get; set; }

    public virtual ICollection<UserRole> UserRoles { get; set; }
}

public class Role
{
    public int Id { get; set; }
    public string Name { get; set; }

    public virtual ICollection<UserRole> UserRoles { get; set; }
}

public class UserRole
{
    public int UserId { get; set; }
    public int RoleId { get; set; }
}

That works just fine, I then wanted to add an IEnumerable<Role> to the user for easier accessibility and for a prettier JSON output. I found an article online which did that like this:

public class User
{
    // All of the previous code
    [NotMapped]
    public virtual IEnumerable<Role> Roles => UserRoles.Select(x => x.Role);
}

I can then get users and roles:

_context.Users.Include(x => x.UserRoles).ThenInclude(y => y.Role)

The thing is, sometimes, I only wish to get the users without the roles:

_context.Users

That makes the program crash, as UserRoles is null, then .Select(x => x.Role) will fail.

My fix to the User class was the following:

public class User
{
    public virtual IEnumerable<Role> Roles
    {
        get
        {
            if (UserRoles == null) return null;
            return UserRoles.Select(x => x.Role);
        }
    }
}

But to me, that is a really hacky and ugly solution to the problem. I just don't know how I can simplify it. I tried doing something like

public virtual IEnumerable<Role> Roles => UserRoles.Select(x => x?.Role);

I want to have something just as simple as the above line, but it should actually work as intended.

Algorythm
  • 130
  • 9
  • Don't trust online articles. Messing up your entity model with not mapped properties which look like navigation properties is confusing and error prone (you can easily try to use them inside the LINQ to Entities query and get surprised). For easier accessibility and for a prettier JSON output, create special classes (a.k.a DTO, ViewModel etc.) and use projection (`Select`) or library like AutoMapper to map the data to/from the entity model. – Ivan Stoev Nov 20 '18 at 20:23
  • @IvanStoev I fixed it with: `public virtual IEnumerable Roles => UserRoles?.Select(ur => ur.Role);`, but you still this doesn't makes sense to have in this entity model? Are you saying that all entity models should have their own DTO, even if they are identical? – Algorythm Nov 20 '18 at 20:34
  • Indeed. And they are not identical. With DTOs/ViewModels you have full control over how the data looks and what is serialized. For instance, the bidirectional entity models (with navigation properties at both side of the relationship) lead to circular problems during JSON serialization. Or unexpected lazy loading (when enabled) at the time it is not expected. Etc. Not to mention the L2E query issues (client evaluation, NRE etc.) – Ivan Stoev Nov 20 '18 at 20:39

2 Answers2

0

Try this:

public virtual IEnumerable<Role> Roles => UserRoles?.Select(x => x.Role);
Dmitry Stepanov
  • 2,776
  • 8
  • 29
  • 45
0

I always do initialize any collection in empty constructor ( for EF ) and always call it from any other. As an example:

 public class User
    {
        public int Id { get; set; }
        public string UserName { get; set; }

        public virtual ICollection<UserRole> UserRoles { get; set; }

        public IEnumerable<Role> Roles => UserRoles.Select(x => x.Role);

        public User()
        {
            this.UserRoles = new List<UserRole>();
        }

        public User(string name)
            :this()
        {

        }
    }

Whenever you will include the roles collection will be fill else you will always have empty collection so any operation wont fail.

Also you do not neet [NotMapped] attribute since the property is readonly and EF will know that

miechooy
  • 3,178
  • 12
  • 34
  • 59