7
\$\begingroup\$

I have a model named User. Each user can have some social links. Now I want to get all users with their social link as a single row in the result. Here are my models:

public class User
{
    public int Id { get; set; }
    public string Name { get; set; }
    public ICollection<SocialLink> SocialLinks { get; set; }
}

public class SocialLink
{
    public string Url { get; set; }
    public SocialType Type { get; set; }
    public User User { get; set; }
    public int UserId { get; set; }
}

public enum SocialType
{
    FaceBook    = 0,
    Twitter     = 1,
    Youtube     = 2,
    Linkedin    = 3,
    GooglePlus  = 4,
    Telegram    = 5,
    Instagram   = 6
}

public class UserViewModel
{
    public string Name { get; set; }
    public string Facebook { get; set; }
    public string Twitter { get; set; }
    public string Youtube { get; set; }
    public string Linkedin { get; set; }
    public string GooglePlus { get; set; }
    public string Telegram { get; set; }
    public string Instagram { get; set; }
}

I want my query to be like this in the result:

Name    | Facebook | Twitter  | Youtube | Linkedin | GooglePlus | Telegram | Instagram
======================================================================================
Someone | facebook | @someone | youtube | linkedin | google     | @tele    | @someins 

I have written following query, It works as I expected but I wonder if there's a better way for doing this:

users.Select(p => new UserViewModel {
            Name = p.Name,
            Facebook = p.SocialLinks.FirstOrDefault(x => x.Type == SocialType.FaceBook).Url ?? "#",
            Twitter = p.SocialLinks.FirstOrDefault(x => x.Type == SocialType.Twitter).Url ?? "#",
            Youtube = p.SocialLinks.FirstOrDefault(x => x.Type == SocialType.Youtube).Url ?? "#",
            Linkedin = p.SocialLinks.FirstOrDefault(x => x.Type == SocialType.Linkedin).Url ?? "#",
            Telegram = p.SocialLinks.FirstOrDefault(x => x.Type == SocialType.Telegram).Url ?? "#",
            Instagram = p.SocialLinks.FirstOrDefault(x => x.Type == SocialType.Instagram).Url ?? "#",
}).ToList();
\$\endgroup\$
  • 1
    \$\begingroup\$ I would create a constructor for UserViewModel that accepts Name and a collection of SocialLink. Inside constructor I would use foreach to populate properties. In your case, every call FirstOrDefault creates an IEnumerator that may slow down your app if there are many users. \$\endgroup\$ – Yuri Tceretian Dec 29 '16 at 15:01
  • 1
    \$\begingroup\$ @YuriTceretian Comments are for seeking clarification. Please post suggestions as answers, not as comments. \$\endgroup\$ – 200_success Dec 29 '16 at 15:18
6
\$\begingroup\$

You could use a Dictionary and make properties of UserViewModel to address elements of this dictionary as follows:

public class UserViewModel
{
    private readonly Dictionary<SocialType, string> SocialLinks;

    public UserViewModel(User user)
    {
        SocialLinks = (user.SocialLinks ?? new SocialLink[0])
                      .ToDictionary(x => x.Type, x => x.Url);
        Name = user.Name;
    }

    private string GetUrl(SocialType socialType)
    {
        string url;
        return SocialLinks.TryGetValue(socialType, out url) && url != null ? url : "#";
    }

    public string Name { get; set; }

    public string Facebook
    {
        get { return GetUrl(SocialType.FaceBook); }
        set { SocialLinks[SocialType.FaceBook] = value; }
    }
    public string Twitter
    {
        get { return GetUrl(SocialType.Twitter); }
        set { SocialLinks[SocialType.Twitter] = value; }
    }
    public string Youtube
    {
        get { return GetUrl(SocialType.Youtube); }
        set { SocialLinks[SocialType.Youtube] = value; }
    }

    ...
}

Then your query will looks like:

users.Select(u => new UserViewModel(u)).ToList();
\$\endgroup\$
  • \$\begingroup\$ Great, By the way Name belongs to User not Social Links: users.Select(u => new UserViewModel(u.SocialLinks, u.Name)).ToList(); \$\endgroup\$ – Sirwan Afifi Dec 29 '16 at 15:08
  • \$\begingroup\$ Even prettier as (socialLinks ?? Enumerable.Empty<SocialLink>).ToDictionary(..) \$\endgroup\$ – t3chb0t Dec 29 '16 at 15:12
  • \$\begingroup\$ I also think using SingleOrDefault would better show that there is always either one link or none if this is not the case then the dictionary will crash trying to insert duplicate keys. \$\endgroup\$ – t3chb0t Dec 29 '16 at 15:14
  • 1
    \$\begingroup\$ Why not pass User and get the Name from that \$\endgroup\$ – paparazzo Dec 29 '16 at 15:15
1
\$\begingroup\$

In my opinion, the current solution is not effective because every time you call FirstOrDefault method it creates enumerator and enumerates until element meets predicate. The SocialLink collection is not expected to be big (0-20 elements) but the collection is users may be big. And creation such a model for 100 users may create some overhead as well as pressure to GC.

The solution with dictionary is more effective than FirstOrDefault, however you have to still deal with possible duplicates and population of dictionary is relatively expensive operation.

I would propose another solution, that seems a little more effective than dictionary, but not so elegant.

public class UserViewModel
{
   public UserViewModel(String name, IEnumerable<SocialLink> links)
   {
      this.Name = name;
      if (links != null){
         foreach(var link in links)
         {
            switch (link.SocialType)
            {
                case SocialType.FaceBook:
                    FaceBook = link.Url;
                    break;
                case SocialType.Twitter:
                    Twitter = link.Url;
                    break;
                case SocialType.Youtube:
                    Youtube = link.Url;
                    break;
                case SocialType.Linkedin:
                    Linkedin = link.Url;
                    break;
                case SocialType.GooglePlus:
                    GooglePlus = link.Url;
                    break;
                case SocialType.Telegram:
                    Telegram = link.Url;
                    break;
                case SocialType.Instagram:
                    Instagram = link.Url;
                    break;
            }
         }
      }
   }

   public string Name { get; set; }
   public string Facebook { get; set; }
   public string Twitter { get; set; }
   public string Youtube { get; set; }
   public string Linkedin { get; set; }
   public string GooglePlus { get; set; }
   public string Telegram { get; set; }
   public string Instagram { get; set; }
}

In this case you create enumerator object and scan collection only once. You can improve to not assign property if it has already a value.

PS. I would change your enum to start from 1, and leave 0 for exceptional cases, because 0 is a default value you may end up with some awkward bugs.

\$\endgroup\$
  • \$\begingroup\$ Appart from the fact that you have a hard-coded switch you've just created a new solution last-or-default :-] \$\endgroup\$ – t3chb0t Dec 29 '16 at 16:03
  • \$\begingroup\$ In this case no, because I create enumerator only once. Logically it is LastOrDefault. \$\endgroup\$ – Yuri Tceretian Dec 29 '16 at 16:04
  • \$\begingroup\$ population of dictionary is relatively expensive operation - this needs a proof. \$\endgroup\$ – t3chb0t Dec 29 '16 at 16:06
  • \$\begingroup\$ I do not see any problems with hard-coded switch in this particular case. Do you? \$\endgroup\$ – Yuri Tceretian Dec 29 '16 at 16:06
  • \$\begingroup\$ @t3chb0t, I would suggest you to look at source code of method Dictionary<T,TR>.Add. It is way more complex than property assignment . \$\endgroup\$ – Yuri Tceretian Dec 29 '16 at 16:08
1
\$\begingroup\$

I think you shouldn't use hard-coded social links as properties on the UserViewModel but instead simplify it to

public class UserViewModel
{
    public string Name { get; set; }
    public IEnumerable<SocialLink> SocialLinks { get; set; }
}

and let the view render a dynamic list of links and format the Url properly for display. The code-behind should not do it.

With this simplification the query becomes

var result = users.Select(p => new UserViewModel
{
    Name = p.Name,
    SocialLinks = 
        p.SocialLinks
        .GroupBy(x => x.Type)
        .Select(g => g.FirstOrDefault())
        .ToList()   
}).ToList();

Nothing is hard-coded anymore. The only thing you need to ever adjust in case of more social link types is probably the view but even this should render just fine without any additional maintenance.

\$\endgroup\$
  • 1
    \$\begingroup\$ I wanted to moan about hardcoding all the types but couldn't be bothered, so glad to see you did ;). I would go further and get rid of the enum, and subclass each social type - surely there are methods you'd want to call on each social link, whose return value should depend on the type. That would get rid of more switch statements down the line. \$\endgroup\$ – 404 Dec 29 '16 at 16:48
  • \$\begingroup\$ @eurotrash definitely an idea worth considering but in this small example I doubt there is much we can do more. \$\endgroup\$ – t3chb0t Dec 29 '16 at 17:06
  • \$\begingroup\$ But the view will likely access the specific property in several places so it would make for more work. \$\endgroup\$ – paparazzo Dec 29 '16 at 18:18
  • \$\begingroup\$ @Paparazzi yes, and display it dynamically as a list because it is one and should be treated as one \$\endgroup\$ – t3chb0t Dec 29 '16 at 18:28
  • \$\begingroup\$ @t3chb0t OP already had a List. To me he is clearly asking for individual properties also. \$\endgroup\$ – paparazzo Dec 29 '16 at 19:04
0
\$\begingroup\$
public class SocialLink
{
    public string Url { get; set; }
    public SocialType Type { get; set; }
    public User User { get; set; }
    public int UserId { get; set; }
}

Why repeat User, and UserId?
Why repeat UserId if User has an Id?

I don't like this

public string Facebook { get; set; }

People are to have and expectation they can revise SocialLinks and it does not.

I would do the work in UserViewModel and derive from User
If you have other selects you would need to repeats the code

List<User> users = new List<User>();
//pop users
List<UserViewModel> usersVM = users.Select(p => new UserViewModel(p)).ToList();

public class User
{
    public int Id { get; set; }
    public string Name { get; set; }
    public ICollection<SocialLink> SocialLinks { get; set; }
    public User(int id, string name, ICollection<SocialLink> socialLinks)
    {
        Id = id;
        Name = name;
        SocialLinks = socialLinks;
    }
    public User(User user)
    {
        Id = user.Id;
        Name = user.Name;
        SocialLinks = user.SocialLinks;
    }
}
public class SocialLink
{
    public string Url { get; set; }
    public SocialType Type { get; set; }
    public SocialLink(string url, SocialType type)
    {
        Url = url;
        Type = type;
    }
}
public class UserViewModel : User
{
    public string Facebook { get; set; }
    public string Twitter { get; set; }
    public string Youtube { get; set; }
    public string Linkedin { get; set; }
    public string GooglePlus { get; set; }
    public string Telegram { get; set; }
    public string Instagram { get; set; }
    public UserViewModel (User user) : base(user)
    {
        PopLinks();
    }
    public UserViewModel(int id, string name, ICollection<SocialLink> socialLinks) : base(id, name, socialLinks)
    {
        PopLinks();
    }
    public void PopLinks()
    {
        if (SocialLinks != null)
        {
            foreach (var link in SocialLinks)
            {   // borrowed from another answer +1
                switch (link.Type)
                {
                    case SocialType.FaceBook:
                        Facebook = link.Url;
                        break;
                    case SocialType.Twitter:
                        Twitter = link.Url;
                        break;
                    case SocialType.Youtube:
                        Youtube = link.Url;
                        break;
                    case SocialType.Linkedin:
                        Linkedin = link.Url;
                        break;
                    case SocialType.GooglePlus:
                        GooglePlus = link.Url;
                        break;
                    case SocialType.Telegram:
                        Telegram = link.Url;
                        break;
                    case SocialType.Instagram:
                        Instagram = link.Url;
                        break;
                }
            }
        }
    }
}  
\$\endgroup\$
  • 1
    \$\begingroup\$ My read is that User might be a navigation property. \$\endgroup\$ – t3chb0t Dec 29 '16 at 15:51
  • \$\begingroup\$ @t3chb0t Yes it is a navigation property. \$\endgroup\$ – Sirwan Afifi Dec 29 '16 at 18:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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