Updating database based on selected check boxes I posted the original question here, but I think it would be nice to have someone else's eye on this code for improving it.

public override void Update(UserProfileDto item)
{
    base.Update(item);

    var memberIntersestIds = _memberInterestService.GetAllPlusDeleted();

    var dbIds = memberIntersestIds.Select(p =&gt; p.InterestSubCategoryId);
    var union = dbIds.ToList().Union(item.InterestIds);

    if (item.InterestIds != null)
    {
        var guids = union as Guid[] ?? union.ToArray();
        foreach (var id in guids)
            if (_memberInterestService.IsSubInterestExist(id))
                _memberInterestService.Delete(id, item.Id, true);

        foreach (var id in guids)
        {
            if (!_memberInterestService.IsSubInterestExist(id))
                _memberInterestService.Add(new MemberInterestDto
                {
                    UserProfileId = item.Id,
                    InterestSubCategoryId = id
                });
            else
                _memberInterestService.Delete(id, item.Id, false);
        }
    }
    else
        foreach (var memberInterest in memberIntersestIds)
            _memberInterestService.Delete(memberInterest.InterestSubCategoryId, item.Id, true);
}


MemberIntersetService

public class MemberInterestService : IMemberInterestService
{
    public IQueryable&lt;MemberInterest&gt; Items { get { return _dbSet.Where(x =&gt; x.IsDeleted == false); } }

    public MemberInterestService(AppDbContext dbContext) : base(dbContext)
    {
        _dbSet = dbContext.MemberInterests;
    }

    public bool IsSubInterestExist(Guid subInterestId)
    {
        return _dbSet.Any(p =&gt; p.InterestSubCategoryId == subInterestId);
    }

    public bool IsSubInterestExistInItem(Guid subInterestId)
    {
        return Items.Any(p =&gt; p.InterestSubCategoryId == subInterestId);
    }

    public void Delete(Guid interestId, Guid userProfileId, bool delete)
    {
        var item = _dbSet.Single(x =&gt; x.InterestSubCategoryId == interestId &amp;&amp; x.UserProfileId == userProfileId);

        //soft delete
        item.IsDeleted = delete;

        _dbSet.Update(item);
    }

    public IList&lt;MemberInterestDto&gt; GetAllPlusDeleted()
    {
        return _dbSet.Select(x =&gt; Mapper.Map&lt;MemberInterest, MemberInterestDto&gt;(x))
            .ToList();
    }
}
OMITTING CURLY BRACES ?

Don't do that a single line statements like this are acceptable even tho they are still not recommended you should always have {} :

if (Foo is Bar)
    return "Foo is Bar";
else
    return "Foo is not Bar";


But this is a huge red flag for bad code style :


foreach (var id in guids)
{
    if (!_memberInterestService.IsSubInterestExist(id))
        _memberInterestService.Add(new MemberInterestDto
        {
            UserProfileId = item.Id,
            InterestSubCategoryId = id
        });
    else
        _memberInterestService.Delete(id, item.Id, false);
}
foreach (var id in guids)
    if (_memberInterestService.IsSubInterestExist(id))
        _memberInterestService.Delete(id, item.Id, true);



If your statement needs to be written on few lines You should have {} e.g in your foreach as it's body contains more than a single line, same goes for your if's it's kinda hard to read it that way and follow the flow of your program.

Aside from that I would move dbIds &amp;&amp; union inside your if (item.InterestIds != null) as you are performing potentially unnecessary operations, they will be used only if the if is triggered anyway : 

if (item.InterestIds != null)
{
    IEnumerable&lt;Guid&gt; dbIds = memberIntersestIds.Select(p =&gt; p.InterestSubCategoryId);
    IEnumerable&lt;Guid&gt; union = dbIds.ToList().Union(item.InterestIds);
    Guid[] guids = union as Guid[] ?? union.ToArray();
    //..
}


This line I assume you have because you are avoiding multiple enumeration


Guid[] guids = union as Guid[] ?? union.ToArray();



But why not call .ToArray() on union instead, there is no need to create new variable here, you are always working with guids and never using union itself as IEnumerable&lt;&gt; :

IEnumerable&lt;Guid&gt; dbIds = memberIntersestIds.Select(p =&gt; p.InterestSubCategoryId);
IEnumerable&lt;Guid&gt; guids = dbIds.ToList().Union(item.InterestIds).ToArray();


This part of your method : 


foreach (var id in guids)
{
    if (_memberInterestService.IsSubInterestExist(id))
    {
        _memberInterestService.Delete(id, item.Id, true);
    }
}

foreach (var id in guids)
{
    if (!_memberInterestService.IsSubInterestExist(id))
    {
        _memberInterestService.Add(new MemberInterestDto
        {
            UserProfileId = item.Id,
            InterestSubCategoryId = id
        });
    }
    else
    {
        _memberInterestService.Delete(id, item.Id, false);
    }
}



It's weird, and so is the .Delete() method. First you have a foreach that remove duplicates from the database


foreach (var id in guids)
{
    if (_memberInterestService.IsSubInterestExist(id))
    {
        _memberInterestService.Delete(id, item.Id, true);
    }
}



But apparently you don't trust your methods and you keep searching if there is any item that exist in your db with a matching ID from the set you've previously iterated and already removed. What's the point ? 

But why you deleting the items from the db only the first time ? What's different ? It's the same ID collection.

Since there is no obvious reason why are you doing this, I would suggest to combine those in a single foreach : 

foreach (var id in guids)
{
    if (_memberInterestService.IsSubInterestExist(id))
    {
        _memberInterestService.Delete(id, item.Id, true);
    }
    else
    {
        _memberInterestService.Add(new MemberInterestDto
        {
            UserProfileId = item.Id,
            InterestSubCategoryId = id
        });
    }
}


With all of those changes your code should  look like this : 

public void Update(UserProfileDto item)
{
    MemberInterestService _memberInterestService = new MemberInterestService();
    IList&lt;MemberInterestDto&gt; memberIntersestIds = _memberInterestService.GetAllPlusDeleted();

    if (item.InterestIds != null)
    {
        IEnumerable&lt;Guid&gt; dbIds = memberIntersestIds.Select(p =&gt; p.InterestSubCategoryId);
        IEnumerable&lt;Guid&gt; guids = dbIds.ToList().Union(item.InterestIds).ToArray();
        foreach (var id in guids)
        {
            if (_memberInterestService.IsSubInterestExist(id))
            {
                _memberInterestService.Delete(id, item.Id, true);
            }
            else
            {
                _memberInterestService.Add(new MemberInterestDto
                {
                    UserProfileId = item.Id,
                    InterestSubCategoryId = id
                });
            }
        }
    }
    else
    {
        foreach (var memberInterest in memberIntersestIds)
        {
            _memberInterestService.Delete(memberInterest.InterestSubCategoryId, item.Id, true);
        }
    }
}


Your Delete method might be better of with a .FirstOrDefault() instead  of .Single() unless you really want to verify there is exactly one item, .FirstOrDefault() should be faster in most cases as it will return whenever the first item is found, where .Single() will always be O(n) operation because it needs to verify if there is exactly one occurrence.

Probably more can be improved, but again more context is required.