Question

I have a method login() which essentially sends a login request to a remote server. As I do not want to waste server resources on processing invalid data, I decided to consider sanity checking the input before I send it.

The code looks something this:

//Method to log user in
public void login(String username, String password) throws IOException{
    //If statement to sanity check credentials
    if (credentialsSanityCheck(username, password)) {
        //Perform login ...
    }
    else {
        throw new IllegalArgumentException("Invalid username/password");
    }
}

However, after learning more about the Single Responsibilty Principle, I am now wondering if it is more appropriate to sanity check the method arguments from the calling code or within the method itself, as it's quite fuzzy (at least for me) to determine whether or not data validation falls under the method's responsibility.

Was it helpful?

Solution

Aspects are odd ducks in SRP (security, logging, etc.). They are often required for a variety of truly valid business reasons, but seem to push a lot of conditional steps in (if the input is safe, then use it to log in, and if something weird happens, log it).

Depending on how pedantic you are, sure, it violates SRP. But, to paraphrase, SRP is not a suicide pact. A somewhat more palatable construction might be making sure both bodies in the conditional are at the same level of abstraction:

//Method to log user in
public void login(String username, String password) throws IOException{
   //If statement to sanity check credentials
   if (credentialsSanityCheck(username, password)) {
      loginWithSafedInput(username, password); // might fail due to user error, locked acct, etc... 
   }
   else {
      raiseUnsafeLogin(username, password); // audit trail and throws
  }
}

The point is to safely log in, with forensic evidence if needed. That seems SRP'y, if you squint.

OTHER TIPS

The login function has one single responsibility: log you in safely, or reject you.

“Safely” is part of its responsibility. That cannot be left to the caller. If you do checks outside the call, login must repeat these checks or it doesn’t even achieve its own responsibility.

SRP is the one if the most misinterpreted principles. You call this “A fair compromise” in a comment. Wrong. SRP doesn’t apply here at all. It often doesn’t apply at all when people think it does who have just learned about it.

Licensed under: CC-BY-SA with attribution
scroll top