How to respect Single responsibility principle and use exception handling at the same time?

Question

I am getting confused on how exactly should I be using exception handling inside a class that respects S-Principle.

For example consider this code in C#:

public class BcryptDecrypt    
{    
    private string _password;
    private string _hash;

    public BcryptDecrypt(string password, string hash)
    {
      this._password = password;
      this._hash = hash;
    }

    public bool Verify()
    {
        return BCrypt.Net.BCrypt.Verify(this._password, this._hash);
    }    
}

Clearly it states that it decrypts a bcrypt hash. But you can see in the Verify method there is no try-catch block to handle an exception.

If we add a try-catch to handle exception then we cannot follow S-Principle. So how do software engineers solve this issue?

// After adding try-catch
public class BcryptDecrypt    
{    
    private string _password;
    private string _hash;

    public BcryptDecrypt(string password, string hash)
    {
      this._password = password;
      this._hash = hash;
    }

    public bool Verify()
    {
        try
        {
            return BCrypt.Net.BCrypt.Verify(this._toDecrypt, 
                                            this._hash);
        }
        catch (System.Exception SomeException)
        {
            // Handle exception as you like and break S-Principle
        }
    }    
}

One way I thought of is to make a separate class to handle each method or class exception or make a universal class to handle it.

I saw similar question on Stack Overflow, but the only given answer said to re-throw the exception to let the controller handle it. This doesn't sound like a solution to me considering the controller itself would be following the S-Principle.

Answer 1

Single responsibility ?

There is a fundamental misunderstanding about SRP here:

• The single responsibility does not mean that a class should do only one thing, but that it should have only one reason to change.

• In other words, it's not the single responsibility OF the class, but the single responsibility FOR the class to change. So it's about decision making.

• Here an enlightening article from Uncle Bob, who invented SOLID, and explains it better than I with all his authority on the subject.

In consequence, you may very well do exception handling in that class without any design concern. You could even have a single Cypher class for encrypting, decrypting and autochecking itself, if you'd really want it.

Do one thing ?

In Clean Code, Uncle Bob also promotes the Do-one-thing-and-do-it-well, for functions. This could be questioned for exception handling:

• Do one thing is not the same as do half of the thing
• If something bad happens when you do the thing, and if you would not handle it as you should, then you would not do the thing well.
• So exception handling does not infringe do-one-thing.

Your option 2 is therefore acceptable.

Anything about try/catch ?

The only remaining principle to consider for your function is Single Level of Abstraction Principle (SLAP):

• if you try on some high level function, you should catch and do some high level function.
• It would not be sound to have a catch clause that would be very low level and detailed, like two page of code for the catch vs 1 line for the try.

If exception handling gets too complex, here a nice pragmatic advice:

It is better to extract the bodies of the try and catch blocks out into functions of their own.

Answer 2

Don't catch exceptions as soon as they are thrown. Catch exceptions at the point you can do something meaningful with them.

But your example code shouldn't be throwing there. The only reasonable exception is ArgumentNullException, which you should throw in the constructor.

public class BcryptDecrypt    
{    
    private string _password;
    private string _hash;

    public BcryptDecrypt(string password, string hash)
    {
      Ensure.ArgumentNotNull(password, nameof(password));
      Ensure.ArgumentNotNull(hash, nameof(hash));

      this._password = password;
      this._hash = hash;
    }

    public bool Verify()
    {
        return BCrypt.Net.BCrypt.Verify(this._password, this._hash);
    }    
}

Ensure.ArgumentNotNull from here.

Answer 3

You didn’t understand the Single Responsibility Principle.

It means: Do one thing - completely. With everything that is needed to make it work. Like exception handling. Adding another class for exception handling would be utterly ridiculous.

A chef cooking a lobster takes a pot, fills it with water, puts it on the oven, turns the heat on, waits until the water is boiling, throws in the lobster, and so on. SRP does not say that you need a PotTakerClass, a PotWithWaterFillerClass, a PotOnOvenPutterClass and another dozen classes.

SRP doesn’t even say you need a LobsterCookerClass. All you need is one ChefClass with the single responsibility to cook food. PS. A good chef is like god, and there's nothing wrong with that.

Answer 4

As others have said, you may be misunderstanding SRP. Don't feel bad though, SRP is not well defined. You would be better off just to ignore it and concentrate on other more well-defined principles like Coupling and Cohesion and the Law of Demeter.

I'm not really here to comment about that though, but about your design. You say:

Clearly it states that it decrypts a bcrypt hash.

It shouldn't state what it does, but what it is. Objects are things, not procedures. So, what it should be is "BcryptHash". And it probably should verify whether a string hashes into it. Something like:

public class BrcyptHash {
   private string _hash;
   ...
   public bool IsOf(string text)
      ...