Skip to main content
deleted 22 characters in body
Source Link
RubberDuck
  • 31.2k
  • 6
  • 74
  • 177
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;
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;
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;
deleted 118 characters in body
Source Link
Mathieu Guindon
  • 75.6k
  • 18
  • 195
  • 469
        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;
        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;
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;
        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;
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;
added 24 characters in body
Source Link
RubberDuck
  • 31.2k
  • 6
  • 74
  • 177
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;
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;
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;
Source Link
RubberDuck
  • 31.2k
  • 6
  • 74
  • 177
Loading