The `FileHeaderValidatorCodeFileReader` rules are arbitrary, and a potential failure point. Hard-coding the validation rules into a bunch of conditions isn't very maintenance-friendly. Create an abstraction to represent a validation rule: public interface IValidationRule { string Name { get; } string Message { get; } bool Evaluate(string[] content); } And then receive an `IEnumerable<IValidationRule>` constructor parameter: private readonly IEnumerable<IValidationRule> _validationRules; public FileHeaderValidatorCodeFileReader(ICodeFileReader reader, IEnumerable<IValidationRule> validationRules) : base(reader) { _validationRules = validationRules; } Now the validation code looks like this: foreach (var rule in _validationRules) { if (!rule.Evaluate(content)) { throw new InvalidFileHeaderException(rule); } } Or with a bit of LINQ, like this: foreach (var failedRule in _validationRules.Where(rule => !rule.Evaluate(content))) { throw new InvalidFileHeaderException(failedRule); } And now you just need a test that validates the the code calls `.Evaluate(content)` on each rule you give it, and a test to validate that the code throws when a rule fails. The rules can be configured at the application's entry point / *composition root*, and modified/refined (/fixed?) without needing to recompile the decorator. --- The `ExceptionLoggerCodeFileReader` is way overboard. Consider: public class ExceptionLoggerCodeFileReader : CodeFileReaderDecorator { private readonly ILogger _logger; private readonly string _message; public ExceptionLoggerCodeFileReader(ICodeFileReader reader, ILogger logger, string message) : base(reader) { _logger = logger; _message = message; } public override string[] ReadFile(string path) { try { return base.Reader.ReadFile(path); } catch (Exception exception) { _logger.ErrorException(_message, exception); throw; } } } The only immediate reason this code might need to change, is to replace `_logger.ErrorException` with a method that produces a log entry at another log level. So be it, it's a trivial change. Why take a `string message` constructor parameter? Because using `exception.Message` in the log entry will likely produce redundant logs when an exception is configured with `ToString` - the log message will be repeated in the string representation of the exception. And move the concern of *coming up with a log message for the specified path*, to the calling code, who already knows about the path. And it's simple as can be, and now you can write a `ShouldLogExceptionAtErrorLevel` test that will break when `_logger.ErrorException` is changed for something else, and a `ShouldRethrow` test that will break if the `throw;` instruction is removed. --- The *Decorator Pattern* is a nice pattern, indeed OCP-friendly. However one needs to consider this: > **Applicability** > > Use Decorator > > - to add responsibilities to individual objects dynamically and transparently, that is, without affecting other objects. > - for responsibilities that can be withdrawn. > - when extension by subclassing is impractical. Sometimes a large number of independent extensions are possible and would produce and explosion of subclasses to support every combination. Or a class definition may be hidden or otherwise unavailable for subclassing. > > <sub>Design Patterns, Elements of Reusable Object-Oriented Software, Structural Patterns / Decorator, p.177</sub> Dynamically adding responsibilities sounds like overkill here, but considering the dependencies are being injected at run-time through IoC, this merely boils down to being able to configure which responsibilities we're giving our `ICodeFileReader` implementation - if the responsibilities need to change, all that needs to change is the IoC configuration. If the *composition root* is in another assembly, then the only assembly that needs a new build is that one, assuming responsibilities are being withdrawn - maybe we no longer want to log exceptions thrown by that class; theres' nothing to change in the implementation, we just tell the IoC container to skip the `ExceptionLoggerCodeFileReader` decorator when resolving an `ICodeFileReader` implementation. Is *decorator* a good design decision *in this case*? Consider this: public class CodeFileReader : ICodeFileReader { private readonly ILogger _logger; private readonly string _message; private readonly IEnumerable<IValidationRule> _validationRules; public CodeFileReader(ILogger logger, string message, IEnumerable<IValidationRule> validationRules) { _logger = logger; _message = message; _validationRules = validationRules; } public string[] ReadFile(string path) { try { var content = File.ReadAllLines(path); foreach (var rule in _validationRules) { if (!rule.Evaluate(content)) { throw new InvalidFileHeaderException(rule); } } return content; } catch (Exception exception) { _logger.ErrorException(_message, exception); throw; } } } This implementation would do exactly the same thing as the bunch of decorators. From a pragmatic point of view, it could be considered a "simpler" approach. From a maintainability point of view though, there's a problem: - As more concerns and responsibilities are added, new dependencies are either added to the constructor for constructor injection, or tightly coupled and `new`'d up directly. As the class grows, its constructor becomes a mess of unrelated parameters. So you start `new`ing things up instead, and introduce coupling and make unit tests depend on things they can't control. - 3 levels of indentation. Both decorator implementations in the OP only needed only 1. - I suppose the code for verifying the file's extension could be inserted before the line where `content` is assigned. - The *intent* of the code is diluted, lost in a bunch of arbitrary checks and features. I'm biased, but I think chosing a Decorator Pattern here has made unit testing much easier, produced cleaner code with much higher cohesion and lower coupling (the "simple" code is tied to a `System.IO.File` static method), and increased readability.