I am trying to adhere to the Single Responsibility Principle (SRP) as much as possible and got used to a certain pattern (for the SRP on methods) heavily relying on delegates. I'd like to know if this approach is sound or if there are any severe issues with it.
For example, to check input to a constructor, I could introduce the following method (the Stream
input is random, could be anything)
private void CheckInput(Stream stream)
{
if(stream == null)
{
throw new ArgumentNullException();
}
if(!stream.CanWrite)
{
throw new ArgumentException();
}
}
This method (arguably) does more than one thing
- Check the inputs
- Throw different exceptions
To adhere to the SRP I therefore changed the logic to
private void CheckInput(Stream stream,
params (Predicate<Stream> predicate, Action action)[] inputCheckers)
{
foreach(var inputChecker in inputCheckers)
{
if(inputChecker.predicate(stream))
{
inputChecker.action();
}
}
}
Which supposedly only does one thing (does it?): Check the input. For the actual checking of the inputs and throwing of the exceptions I have introduced methods like
bool StreamIsNull(Stream s)
{
return s == null;
}
bool StreamIsReadonly(Stream s)
{
return !s.CanWrite;
}
void Throw<TException>() where TException : Exception, new()
{
throw new TException();
}
and can call CheckInput
like
CheckInput(stream,
(this.StreamIsNull, this.Throw<ArgumentNullException>),
(this.StreamIsReadonly, this.Throw<ArgumentException>))
Is this any better than the first option at all, or do I introduce unneccesary complexity? Is there any way I can still improve this pattern, if it's viable at all?
CheckInput
is still doing multiple things: It is both iterating over an array and calling a predicate function and calling a action function. Is that then not a violation of the SRP?