6
\$\begingroup\$

Long story short a user can have things in common with another user, but I don't what to show more than 4 main categories in common, only the first 4, the priority is the order in which the lists are added.

Below is my current approach, but I feel that this can be simplified, and maybe improve the time.

Any thoughts?

var user1 = await UserService.GetInfo(id, reqUserId);
var user2 = await UserInfoService.GetInfo(reqUserId, reqUserId);

// determine info
List<InfoItemVM> infos = new List<InfoItemVM>();
InfoVM info = new InfoVM();

infos.Add( new InfoItemVM { Name = "Projects", Data = user1.Projects.Intersect(user2.Projects).ToList() });
infos.Add( new InfoItemVM { Name = "Companies", Data = user1.Companies.Intersect(user2.Companies).ToList() });
infos.Add( new InfoItemVM { Name = "Schools", Data = user1.Schools.Intersect(user2.Schools).ToList() });
infos.Add( new InfoItemVM { Name = "Hobbies", Data = user1.Hobbies.Intersect(user2.Hobbies).ToList() });
infos.Add( new InfoItemVM { Name = "Locations", Data = user1.Locations.Intersect(user2.Locations).ToList() });
infos.Add( new InfoItemVM { Name = "Interests", Data = user1.Interests.Intersect(user2.Interests).ToList() });
infos.Add( new InfoItemVM { Name = "Stuff", Data = user1.Stuff.Intersect(user2.Stuff).ToList() });
infos.Add( new InfoItemVM { Name = "Things", Data = user1.Things.Intersect(user2.Things).ToList() });
infos.Add( new InfoItemVM { Name = "MyLists", Data = user1.MyLists.Intersect(user2.MyLists).ToList() });

// assign bullet classes to maximum 4 in info categories
int max = 0; int med = 0; int medsm = 0; int min = 0;
int count = 0;

foreach (var c in infos.Where(c => c.Data.Count != 0))
{
    count++;
    if (count > 4 && c.Data.Count != 0)
        c.Data.Clear();
}

foreach (var c in infos)
{
    int value = c.Data.Count;
    if (value > max)
    {
        med = max;
        medsm = max;
        min = max;
        max = value;
    }
    else if (value < max && value > med)
    {
        medsm = med;
        min = med;
        med = value;
    }
    else if (value < med && value > medsm)
    {
        min = medsm;
        medsm = value;
    }
}
for (int i = 0; i < infos.Count; i++)
{
    int value = infos[i].Data.Count;
    // Doing this in order to return an object
    switch (infos[i].Name)
    {
        case "Projects":
            info.Projects.Data = infos[i].Data;
            info.Projects.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        case "Companies":
            info.Companies.Data = infos[i].Data;
            info.Companies.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        case "Schools":
            info.Schools.Data = infos[i].Data;
            info.Schools.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        case "Hobbies":
            info.Hobbies.Data = infos[i].Data;
            info.Hobbies.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        case "Locations":
            info.Locations.Data = infos[i].Data;
            info.Locations.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        case "Interests":
            info.Interests.Data = infos[i].Data;
            info.Interests.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        case "Stuff":
            info.Stuff.Data = infos[i].Data;
            info.Stuff.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        case "Things":
            info.Things.Data = infos[i].Data;
            info.Things.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        case "MyLists":
            info.MyLists.Data = infos[i].Data;
            info.MyLists.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        default:
            break;
    }
}
return Ok(info);


public class InfoVM
{
    public InfoItemVM  Projects { get; set; }
    public InfoItemVM  Companies { get; set; }
    public InfoItemVM  Schools { get; set; }
    public InfoItemVM  Hobbies { get; set; }
    public InfoItemVM  Locations { get; set; }
    public InfoItemVM  Interests { get; set; }
    public InfoItemVM  Stuff { get; set; }
    public InfoItemVM  Things { get; set; }
    public InfoItemVM  MyLists { get; set; }

    public InfoVM()
    {
        Projects = new InfoItemVM  { Data = new List<string>(), Bullet = "", Name = "" };
        PreviousCompanies = new InfoItemVM  { Data = new List<string>(), Bullet = "", Name = "" };
        Schools = new InfoItemVM  { Data = new List<string>(), Bullet = "", Name = "" };
        Hobbies = new InfoItemVM  { Data = new List<string>(), Bullet = "", Name = "" };
        Locations = new InfoItemVM  { Data = new List<string>(), Bullet = "", Name = "" };
        Interests = new InfoItemVM  { Data = new List<string>(), Bullet = "", Name = "" };
        Stuff = new InfoItemVM  { Data = new List<string>(), Bullet = "", Name = "" };
        Things = new InfoItemVM  { Data = new List<string>(), Bullet = "", Name = "" };
        MyLists = new InfoItemVM  { Data = new List<string>(), Bullet = "", Name = "" };
    }
}

public class InfoItemVM 
{
    public string Name { get; set; }
    public List<string> Data { get; set; }
    public string Bullet { get; set; }

    public InfoItemVM()
    {
        Data = new List<string>();
    }
}
\$\endgroup\$
2
  • \$\begingroup\$ Can you add InfoVM implementation? \$\endgroup\$
    – Nikita B
    Commented Jan 16, 2017 at 15:37
  • \$\begingroup\$ @NikitaB sure, done \$\endgroup\$
    – Dimitri
    Commented Jan 16, 2017 at 15:51

3 Answers 3

5
\$\begingroup\$

Every single case has the same code, and it's easy to replace it.

We break it down into methods so that the code is more isolated and easier to debug (and test).

public void UpdateItem(InfoItemVM item, List<string> data, int max, int med, int medsm)
{
    int value = infos[i].Data.Count;
    item.Data = data;
    item.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
}

public void UpdateItem(InfoVM viewModel, InfoItemVM item, int max, int med, int medsm)
{ 
    switch (item.Name)
    {
        case "Projects":
            UpdateItem(viewModel.Projects, item.Data, max, med, medsm);
            break;
        case "Companies":
            UpdateItem(viewModel.Companies, item.Data, max, med, medsm);
            break;
        case "Schools":
            UpdateItem(viewModel.Schools, item.Data, max, med, medsm);
            break;
        case "Hobbies":
            UpdateItem(viewModel.Hobbies, item.Data, max, med, medsm);
            break;
        case "Locations":
            UpdateItem(viewModel.Locations, item.Data, max, med, medsm);
            break;
        case "Interests":
            UpdateItem(viewModel.Interests, item.Data, max, med, medsm);
            break;
        case "Stuff":
            UpdateItem(viewModel.Stuff, item.Data, max, med, medsm);
            break;
        case "Things":
            UpdateItem(viewModel.Things, item.Data, max, med, medsm);
            break;
        case "MyLists":
            UpdateItem(viewModel.MyLists, item.Data, max, med, medsm);
            break;
    }
}

public void UpdateItems(InfoVM viewModel, List<InfoItemVM> items, int max, int med, int medsm)
{
    foreach (var item in items)
    {
        UpdateItem(viewModel, item, max, med, medsm);
    }
}

Then this:

for (int i = 0; i < infos.Count; i++)
{
    int value = infos[i].Data.Count;
    // Doing this in order to return an object
    switch (infos[i].Name)
    {
        case "Projects":
            info.Projects.Data = infos[i].Data;
            info.Projects.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        case "Companies":
            info.Companies.Data = infos[i].Data;
            info.Companies.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        case "Schools":
            info.Schools.Data = infos[i].Data;
            info.Schools.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        case "Hobbies":
            info.Hobbies.Data = infos[i].Data;
            info.Hobbies.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        case "Locations":
            info.Locations.Data = infos[i].Data;
            info.Locations.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        case "Interests":
            info.Interests.Data = infos[i].Data;
            info.Interests.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        case "Stuff":
            info.Stuff.Data = infos[i].Data;
            info.Stuff.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        case "Things":
            info.Things.Data = infos[i].Data;
            info.Things.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        case "MyLists":
            info.MyLists.Data = infos[i].Data;
            info.MyLists.Bullet = value == max ? "big" : value == med ? "medium" : value == medsm ? "medium-small" : "small";
            break;
        default:
            break;
    }
}

Simply becomes:

UpdateItems(info, infos, max, med, medsm);

Of course, we can take one more step to cleaning this up by using a Dictionary<string, InfoItemVM>:

Take our UpdateItem(InfoVM, InfoItemVM, int, int, int) method (second one) from above and rewrite it as follows:

public void UpdateItem(InfoVM viewModel, InfoItemVM item, int max, int med, int medsm)
{ 
    var map = new Dictionary<string, InfoItemVM>
    {
        {"Projects", viewModel.Projects},
        {"Companies", viewModel.Companies},
        {"Schools", viewModel.Schools},
        {"Hobbies", viewModel.Hobbies},
        {"Locations", viewModel.Locations},
        {"Interests", viewModel.Interests},
        {"Stuff", viewModel.Stuff},
        {"Things", viewModel.Things},
        {"MyLists", viewModel.MyLists},
    };

    UpdateItem(map[item.Name], item.Data, max, med, medsm);
}
\$\endgroup\$
2
  • \$\begingroup\$ Hello, and than you for your interest. I think you meant UpdateItem(map[item.Name], item.Data, max, med, medsm); maybe \$\endgroup\$
    – Dimitri
    Commented Jan 17, 2017 at 8:33
  • \$\begingroup\$ @SGN Yep, good catch. \$\endgroup\$ Commented Jan 17, 2017 at 15:20
8
\$\begingroup\$

You should replace individual properties on InfoVM with a collection. Then you can use LINQ to build it:

var commonCategories = infos.Where(x => x.Data.Count > 0)
                            .Take(4)
                            .OrderBy(x => x.Data.Count);

var index = 0;                            
foreach(var category in commonCategories)
{
    category.Bullet = (BulletSize) index++;
    info.CommonCategories .Add(category);
}

where

enum BulletSize
{
    Small,
    MediumSmall,
    Medium,
    Large
}

public class InfoVM
{
    public ObservableCollection<InfoItemVM> CommonCategories { get; set; }

    ....
}
\$\endgroup\$
2
  • \$\begingroup\$ The problem with this is that the OP is required to replace the view model, which may not be possible in this scenario. \$\endgroup\$ Commented Jan 16, 2017 at 16:06
  • 2
    \$\begingroup\$ @EBrown, true. But this problem is not going away unless InfoVM is changed. Because current design is the problem. \$\endgroup\$
    – Nikita B
    Commented Jan 16, 2017 at 16:14
1
\$\begingroup\$

The Name property is too fragile
If an invalid Name is entered then lots of stuff breaks
You have a tight coupling to InfoItemVM properties

public class InfoItemVM 
{
    public string Name { get; set; }
    public List<string> Data { get; set; }
    public string Bullet { get; set; }

    public InfoItemVM()
    {
        Data = new List<string>();
    }
}

When you initialize you don't even give proper names

Projects = new InfoItemVM  { Data = new List<string>(), Bullet = "", Name = "" };
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.