Create User profile with separate social media data

Question

I am creating user profile logic for a blogging platform. I have the UserProfile model and then two separate models, UserSocialLink and SocialLinkType to control the user's various social media contacts (i.e. Facebook, Twitter, Google+, etc...). I am looking for feed back on the execution of the methods used to Add new UserProfiles. Is this the way you would handle it? Is there a better way? What are some of the issues you see with my methods (specifically in the service class)?

The Models

public class UserProfile
{
    public int Id { get; set; }
    public string UserName { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string ProfilePicUrl { get; set; }
    public string Location { get; set; }
    public string DateOfBirth { get; set; }
    public string Bio { get; set; }
    public DateTime Created { get; set; }
    public DateTime Updated { get; set; }
 }

public class UserSocialLink
{
    public int Id {get; set;}
    public int UserProfileId { get; set; }
    public int SocialLinkTypeID { get; set; }
    public string LinkValue { get; set; }
    public DateTime Created { get; set; }
    public DateTime Updated { get; set; }

    public virtual UserProfile UserProfile { get; set; }
    public virtual SocialLinkType SocialLinkType {get; set;}
}

public class SocialLinkType
{
    public int Id { get; set; }
    public string LinkType { get; set; }
    public string URLFormat { get; set; }
}

The Add Methods in The Service Class

public class UserProfileService : IUserProfileService
{
    public UserProfile Add(UserProfile userProfile)
    {
        var now = DateTime.UtcNow;

        if (userProfile == null)
            throw new ArgumentNullException("User Profile");

        userProfile.Created = now;
        userProfile.Updated = now;
        using (var db = new ApplicationDbContext())
        {
            db.UserProfiles.Add(userProfile);
            db.SaveChanges();
        }
        return userProfile;
    }

    public UserProfile AddSocialLink(string userName, UserSocialLink userSocialLink, string socialLinkType)
    {
        var now = DateTime.UtcNow;
        var userProfile = Get(userName);
        if (userProfile == null)
            throw new Exception("User Profile Not Found");
        using (var db = new ApplicationDbContext())
        {
            var linkType = db.SocialLinkTypes.SingleOrDefault(p => p.LinkType.ToUpper() == socialLinkType.ToUpper());
            if (linkType == null)
                throw new Exception("Social Media Type Not Found");
            userSocialLink.Created = now;
            userSocialLink.Updated = now;
            userSocialLink.SocialLinkTypeID = linkType.Id;
            db.UserSocialLinks.Add(userSocialLink);
            db.SaveChanges();
            return userProfile;
         }
    }
}

Related GitHub Issue #14

Answer 1

In addition RubberDuck's answer about the braces:

Inside the AddSocialLink() method you have this

var userProfile = Get(userName);
if (userProfile == null)
    throw new Exception("User Profile Not Found");

Here, with the usage of var, it isn't obvious what datatype is assigned to userProfile. The methodname Get does not reflect anything to make it more clear.
Sure, Mr.Maintainer could assume, as this is the UserProfileService, that the return type would be a UserProfile, but it would be more explicit to add a ExistUserProfile() method which just returns a boolean.

private Boolean ExistUserProfile(String userName)
{
    UserProfile userProfile = Get(userName);
    return userProfile != null;
}

The UserProfileService class does violate the Single responsibility principle as it is responsible to add, get etc UserProfile data and at least to also add UserSocialLink data.
This is also reflected clearly in the question title: "Create User profile with separate social media data".

Returning a UserProfile object from this method does not make any sense, as the code isn't changing any property of the retrieved userprofile.
A user of the UserProfileService class wouldn't expect that adding a SocialLink by calling the AddSocilaLink() method would return a UserProfile object.

Answer 2

public UserProfile AddSocialLink(string userName, UserSocialLink userSocialLink, string socialLinkType)

Shouldn't the socialLinkType be a property of the UserSocialLink class? It seems odd to me that the code has to pass a string along with the object in order to tell the method what type it is. That said, I really do like that you're checking the database to make sure the type is valid. Now that I think of it, why pass the user name as a string? Wouldn't it be a simpler API to pass a UserProfile in? At least, it would be more consistent for the dev using the UserProfileService.

---

This code is a great example of why we should all use proper indentation and braces around if statements.

var linkType = db.SocialLinkTypes.SingleOrDefault(p => p.LinkType.ToUpper() == socialLinkType.ToUpper());
if (linkType == null)
    throw new Exception("Social Media Type Not Found");
userSocialLink.Created = now;
userSocialLink.Updated = now;
userSocialLink.SocialLinkTypeID = linkType.Id;
db.UserSocialLinks.Add(userSocialLink);
db.SaveChanges();
return userProfile;

At a glance, it looks like the userSocialLink only gets it's properties set if (linkType == null) and after an exception is thrown. Of course, this is ridiculous and not what is actually happening, but braces make that crystal clear to Mr. Maintainer.

var linkType = db.SocialLinkTypes.SingleOrDefault(p => p.LinkType.ToUpper() == socialLinkType.ToUpper());
if (linkType == null)
{
    throw new Exception("Social Media Type Not Found");
}
userSocialLink.Created = now;
userSocialLink.Updated = now;
userSocialLink.SocialLinkTypeID = linkType.Id;
db.UserSocialLinks.Add(userSocialLink);
db.SaveChanges();
return userProfile;