Blog

  • Home /
  • Blog /
  • Refactoring Exercise: The Single Responsibility Principle vs Needless Complexity

Refactoring Exercise: The Single Responsibility Principle vs Needless Complexity

October 6, 2008

Ray Houston has written this post on his blog named Single-Responsibility Versus Needless Complexity. His post contains the following code sample of which he suspects that it possibly violates the Single Responsibility Principle:

public bool Login(string username, string password)
{
    var user = userRepo.GetUserByUsername(username);

    if(user == null)
        return false;

    if (loginValidator.IsValid(user, password))
        return true;

    user.FailedLoginAttempts++;

    if (user.FailedLoginAttempts >= 3)
        user.LockedOut = true;

    return false;
}

Because I’m still sitting at home slowly recovering from surgery (almost a month now), and because I’m terribly bored by now, I thought I’d just try to refactor this code. The first step I took was to isolate the validation of the user in a separate private method like so:

private bool IsValid(User user, String password)
{
    if(user == null)
        return false;
    
    return loginValidator.IsValid(user, password);
}

This slightly simplifies the code of the Login method:

public bool Login(string username, string password)
{
    var user = userRepo.GetUserByUsername(username);
    if (IsValid(user, password))
        return true;

    user.FailedLoginAttempts++;

    if (user.FailedLoginAttempts >= 3)
        user.LockedOut = true;

    return false;
}

Next thing I noticed was the use of the FailedLoginAttempts and the LockedOut properties, which appear to be part of the User class. This is something I talked about in Properties - A False Sense of Encapsulation. Let us apply Tell, Don’t Ask and move this code into the User class like so: 

public bool Login(string username, string password)
{
    var user = userRepo.GetUserByUsername(username);
    if (IsValid(user, password))
        return true;

    user.FailedLoginAttempts++;

    if (user.FailedLoginAttempts >= 3)
        user.LockedOut = true;

    return false;
}

This eliminates the need of the properties I just mentioned, which expose private data of the User class (at least for the Login story). The code of the Login method now becomes fairly small:

public bool Login(string username, string password)
{
    var user = userRepo.GetUserByUsername(username);
    if (IsValid(user, password))
        return true;

    user.FailedLoginAttempts++;

    if (user.FailedLoginAttempts >= 3)
        user.LockedOut = true;

    return false;
}

Besides good judgement, SRP is also about organizing complexity so that other developers/readers know where to look for it.

Now get those torches lit and flame away.

If you and your team want to learn more about how to write maintainable unit tests and get the most out of TDD practices, make sure to have look at our trainings and workshops or check out the books section. Feel free to reach out at infonull@nullprincipal-itnull.be.

Profile picture of Jan Van Ryswyck

Jan Van Ryswyck

Thank you for visiting my blog. I’m a professional software developer since Y2K. A blogger since Y2K+5. Provider of training and coaching in XP practices. Curator of the Awesome Talks list. Past organizer of the European Virtual ALT.NET meetings. Thinking and learning about all kinds of technologies since forever.

Comments

Latest articles

Tags

Disclaimer

The opinions expressed on this blog are my own personal opinions. These do NOT represent anyone else’s view on the world in any way whatsoever.

About

Thank you for visiting my website. I’m a professional software developer since Y2K. A blogger since Y2K+5. Author of Writing Maintainable Unit Tests. Provider of training and coaching in XP practices. Curator of the Awesome Talks list. Thinking and learning about all kinds of technologies since forever.

Latest articles

Contact information

(+32) 496 38 00 82

infonull@nullprincipal-itnull.be

principal-it.eu
principal-it.be
janvanryswyck.com