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;
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;