CODEWITHSUNDEEP
c#linqentity-frameworkmulti-select
akd
akd

Reputation: 6740

How can I simplify this LINQ code

Basically I have a multiselect list box in MVC and when the user changes selection it will come back and it should update the model. the below code works but I am just wondering how can I put it in a single foreach loop or is there a better way of updating the selection? Note: There is a many to many relationship between artist and artist type.

foreach (var artistTtype in this._db.ArtistTypes.ToList().Where(artistTtype => artist.ArtistTypes.Contains(artistTtype)))
{
    artist.ArtistTypes.Remove(artistTtype);
}

foreach (var artistTtype in this._db.ArtistTypes.ToList().Where(artisttype => vm.SelectedIds.Contains(artisttype.ArtistTypeID)))
{
    artist.ArtistTypes.Add(artistTtype);
}

Upvotes: 1

Views: 95

Answers (2)

Kamil Budziewski
Kamil Budziewski

Reputation: 23107

This for adding (just use AddRange):

artist.ArtistTypes.AddRange(this._db.ArtistTypes
         .Where(artisttype => vm.SelectedIds.Contains(artisttype.ArtistTypeID)));

This for removing (use ForEach):

 this._db.ArtistTypes
         .Where(artistTtype => artist.ArtistTypes.Contains(artistTtype)).ToList()
              .ForEach(x=>artist.ArtistTypes.Remove(x));

EDIT:

you can always set 

artist.ArtistTypes = this._db.ArtistTypes
         .Where(artisttype => vm.SelectedIds.Contains(artisttype.ArtistTypeID)).ToList();

this will set ArtistTypes to what you want, you don't need to delete then add.

Upvotes: 2

Alex
Alex

Reputation: 23300

I see two "fixes":

1) You don't need to care about what's inside the list, since you're going to update the list of selections you can start from scratch, so the removal part becomes

artist.ArtistTypes.Clear();

2) Now you fill the list again. ToList() should not be needed since you're performing a .Where() to get the data, and you can leverage Linq's lazy mechanisms so you'll only read the data you actually use. You can also split the lines for increased readability (it doesn't matter: until you do the foreach() the db will not be actually hit.

//note that the .ToList() is gone
var query = this._db.ArtistTypes.Where(artisttype => vm.SelectedIds.Contains(artisttype.ArtistTypeID);
foreach (var artistTtype in query))
{
    artist.ArtistTypes.Add(artistTtype);
}

2b) (UNTESTED, off the top of my head) Another way of implementing the comparison you do is through a custom IEqualityComparer, switching to .Intersect() method. This is way more solid since if your keys change in the model you only have to change the comparer.

// I'm making up "ArtistType", fix according to your actual code
class ArtistTypeEqualityComparer : IEqualityComparer<ArtistType>
{
    public bool Equals(ArtistType x, ArtistType y)
    {
        if (ArtistType.ReferenceEquals(x, null)) return false;
        if (ArtistType.ReferenceEquals(y, null)) return false;
        if (ArtistType.ReferenceEquals(x, y)) return true;

        return x.ArtistTypeId.Equals(y.ArtistTypeId);
    }
    public int GetHashCode(ArtistType obj)
    {
        return obj.ArtistTypeId.GetHashCode();
    }
}

// And then the "add" part simplifies

artist.ArtistTypes.AddRange(this._db.ArtistTypes.Intersect(vm.SelectedIds.Select(x => new ArtistType{ ArtistTypeId = x }));

Upvotes: 0

Related Questions