ListBox Update Handling

Question

I have UserGroups for Users to be assigned to in a ListBox in HTML form. Items from listbox can be multi-selected or completely unselected.

In summary currently, I am getting a int[] of GroupId from client into my action and I am pulling existing assigned usergroups for the user into the action as well. Then I am comparing the Length of both int[] to determine whether UserGroups for the User was added or removed which is depicted in the following flow chart:

flow_chart

Actual code:

var existUserGroups = db.Users.Where(u => u.ID == user.ID).Single().UserGroups;
var existUserGroupsIds = db.Users.Where(u => u.ID == user.ID).Single().UserGroups.Select(ug => ug.ID).ToArray<int>();
var newUserGroupsIds = user.GroupId ?? new int[0];
//No change made
if (!existUserGroupsIds.SequenceEqual(newUserGroupsIds))
{
 //Add
 if (newUserGroupsIds.Length > existUserGroupsIds.Length)
 {
 for (int i = 0; i < newUserGroupsIds.Length; i++)
 {
 if (!existUserGroupsIds.Any(id => id == newUserGroupsIds[i]))
 {
 int id = newUserGroupsIds[i];
 existUserGroups.Add(db.UserGroups.Where(ug => ug.ID == id).Single());
 }
 }
 }
 //Remove
 else if (newUserGroupsIds.Length < existUserGroupsIds.Length)
 {
 var diff = existUserGroupsIds.Except(newUserGroupsIds);
 if (diff != null && diff.Count() > 0)
 {
 for (int i = 0; i < diff.Count(); i++)
 {
 int id = diff.ElementAt(i);
 existUserGroups.Remove(db.UserGroups.Where(ug => ug.ID == id).Single());
 }
 }
 }
 //Update
 else
 {
 existUserGroups.Remove(existUserGroups.ElementAt(0));
 int id = newUserGroupsIds[0];
 existUserGroups.Add(db.UserGroups.Where(ug => ug.ID == id).Single());
 }
 db.SaveChanges();
}

This is working but the code seems to be very tangled. So, please suggest how I can improve this code or how devs usually handle such situations.

• Ben Aaronson: Just want to check I'm guessing correctly: you're using an ORM like Entity Framework, Users and Groups are each entities with their own table, and because they're in a many-to-many relationship you also have a UserGroups table to map them. Is that correct?
• lbrahim: @BenAaronson You guessed correct! But EF handles the join table stuff so all I have for typed models are User and Group.
• Ben Aaronson: So db.UserGroup is what you're calling a Group?
• lbrahim: @BenAaronson Yeah, yeah. Sorry for the confusion. Technically, typed models are UserGroup and User. I will edit my question.

Answer 1

Small Code duplication

var existUserGroups = db.Users.Where(u => u.ID == user.ID).Single().UserGroups;
var existUserGroupsIds = db.Users.Where(u => u.ID == user.ID).Single().UserGroups.Select(ug => ug.ID).ToArray<int>();

can be reduced to

var existUserGroups = db.Users.Where(u => u.ID == user.ID).Single().UserGroups;
var existUserGroupsIds = existUserGroups.Select(ug => ug.ID).ToArray<int>();

The //Add part

Should be simplified using a foreach loop and made more readable by using the Contains() method

foreach (int newId in newUserGroupsIds)
{
 if (!existUserGroupsIds.Contains(newId))
 {
 existUserGroups.Add(db.UserGroups.Where(ug => ug.ID == newId).Single());
 }
}

The //Remove part

var diff = existUserGroupsIds.Except(newUserGroupsIds);

diff will never be null => the null check can be skipped
using a for..each instead of a for loop => the check for diff.Count() > 0 can be skipped

var diff = existUserGroupsIds.Except(newUserGroupsIds);
foreach (int id in diff)
{
 existUserGroups.Remove(db.UserGroups.Where(ug => ug.ID == id).Single());
}