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\$
2
  • 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\$ Dec 29, 2016 at 15:01
  • 1
    \$\begingroup\$ @YuriTceretian Comments are for seeking clarification. Please post suggestions as answers, not as comments. \$\endgroup\$ Dec 29, 2016 at 15:18

4 Answers 4

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\$
4
  • \$\begingroup\$ Great, By the way Name belongs to User not Social Links: users.Select(u => new UserViewModel(u.SocialLinks, u.Name)).ToList(); \$\endgroup\$ Dec 29, 2016 at 15:08
  • \$\begingroup\$ Even prettier as (socialLinks ?? Enumerable.Empty<SocialLink>).ToDictionary(..) \$\endgroup\$
    – t3chb0t
    Dec 29, 2016 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, 2016 at 15:14
  • 1
    \$\begingroup\$ Why not pass User and get the Name from that \$\endgroup\$
    – paparazzo
    Dec 29, 2016 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\$
9
  • \$\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, 2016 at 16:03
  • \$\begingroup\$ In this case no, because I create enumerator only once. Logically it is LastOrDefault. \$\endgroup\$ Dec 29, 2016 at 16:04
  • \$\begingroup\$ population of dictionary is relatively expensive operation - this needs a proof. \$\endgroup\$
    – t3chb0t
    Dec 29, 2016 at 16:06
  • \$\begingroup\$ I do not see any problems with hard-coded switch in this particular case. Do you? \$\endgroup\$ Dec 29, 2016 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\$ Dec 29, 2016 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\$
5
  • 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, 2016 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, 2016 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, 2016 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, 2016 at 18:28
  • \$\begingroup\$ @t3chb0t OP already had a List. To me he is clearly asking for individual properties also. \$\endgroup\$
    – paparazzo
    Dec 29, 2016 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\$
2
  • 1
    \$\begingroup\$ My read is that User might be a navigation property. \$\endgroup\$
    – t3chb0t
    Dec 29, 2016 at 15:51
  • \$\begingroup\$ @t3chb0t Yes it is a navigation property. \$\endgroup\$ Dec 29, 2016 at 18:05

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.