Composition Over Inheritance: Refactoring a .NET Class Hierarchy
A realistic inheritance hierarchy that rots under new requirements, and a step-by-step refactor into composed behaviours that stay easy to change and test.
4 min readOriginal article by Vichea Nath
Some of the old .NET Framework Legacy Systems that i been working on more likely i saw it has one base class controller or service that has may common fuction like a helper function.
The starting point
A notification feature. The first version has one channel and one base class, which is entirely reasonable:
csharp
publicabstractclassNotifier{protectedabstractTaskSendAsync(string recipient,string body);publicasyncTaskNotifyAsync(Order order){var body =quot;Order {order.Id} is now {order.Status}.";awaitSendAsync(order.CustomerContact, body);}}publicclassEmailNotifier:Notifier{protectedoverrideTaskSendAsync(string recipient,string body)=> _smtp.SendAsync(recipient,"Order update", body);}
Then the requirements arrive, one release at a time:
Add SMS.
Some notifications need retries; some must not be retried.
Marketing wants templated bodies for email but plain text for SMS.
Audit needs a log entry for every send, except test sends.
High-value orders must notify both email and Slack.
What the hierarchy looks like six months later
csharp
publicabstractclassNotifier{protectedbool ShouldRetry {get;set;}=true;protectedbool SkipAudit {get;set;}protectedvirtualbool UsesTemplates =>false;publicasyncTaskNotifyAsync(Order order){var body = UsesTemplates
?awaitRenderTemplateAsync(order):quot;Order {order.Id} is now {order.Status}.";var attempts = ShouldRetry ?3:1;for(var i =0; i < attempts; i++){try{awaitSendAsync(order.CustomerContact, body);break;}catch(Exception)when(i < attempts -1){await Task.Delay(200*(i +1));}}if(!SkipAudit){await _audit.RecordAsync(order.Id,GetType().Name);}}protectedabstractTaskSendAsync(string recipient,string body);protectedvirtualTask<string>RenderTemplateAsync(Order order)=>thrownewNotSupportedException();}publicclassEmailNotifier:Notifier{/* ... */}publicclassTemplatedEmailNotifier:EmailNotifier{protectedoverridebool UsesTemplates =>true;}publicclassSmsNotifier:Notifier{publicSmsNotifier()=> ShouldRetry =false;}publicclassTestSmsNotifier:SmsNotifier{publicTestSmsNotifier()=> SkipAudit =true;}publicclassEmailAndSlackNotifier:EmailNotifier{/* sends twice, somehow */}
Nothing here is written by a careless developer we all have full of knowledge and experience. Every line was a reasonable local decision. The problem is structural.
Diagram
Rendering diagram…
The symptoms, named
Protected flags are configuration in disguise.ShouldRetry and SkipAudit exist so subclasses can switch off parts of the base class. A subclass that turns off half of its parent is not a specialization; it is a different object wearing the parent's constructor.
NotSupportedException in a virtual method breaks the type.RenderTemplateAsync says every notifier can render a template, and then some of them throw. Any code holding a Notifier now has to know which concrete type it really has.
One base class owns four responsibilities. Message building, retry policy, transport, and auditing all live in NotifyAsync. Changing retry behaviour means editing a class that every notifier inherits from, and re-testing all of them.
Combinations multiply subclasses. "Templated + no retry + no audit + two transports" has no home in a tree. You either add another leaf class or add another flag.
Tests are heavy. To test retry behaviour you must instantiate a concrete notifier, which drags in SMTP or an SMS gateway. it seem many case need to handle
name the behaviours as roles
Before writing any new class, list what the base class actually does, and give each one an interface:
If you out of idea how to call or how to name those things just throw it in a one of your AI like ChatGPT and ask for a name suggestion. they are very good at it.
The "email and Slack" case that needed its own subclass is now a collection with two entries. "No audit in tests" is a NullAuditLog. "No retry for SMS" is NoRetry. None of these require a new type in a hierarchy.
Diagram
Rendering diagram…
move the variation into registration
The combinations that used to be subclasses now live in one readable place:
A fake IChannel is a handful of lines because the interface has one method. That is the practical payoff: narrow interfaces make honest test doubles cheap.
Doing this to existing code safely
You rarely get to rewrite the tree in one commit. A sequence that works:
Add characterization tests against the current public behaviour, whatever it is.
Extract one role — retry is usually the easiest — into an interface plus implementation, and have the base class delegate to it. Nothing else changes yet.
Repeat per responsibility until the base class only orders steps.
Flip the relationship: turn the base class into a standalone class that takes the roles as constructor parameters.
Delete the subclasses, replacing each one with a registration.
Seal what remains. If a class is not designed for inheritance, say so in the type system.
Each step compiles, ships, and is individually revertible.
When inheritance is still the right call
Composition is a default, not a rule. Inheritance earns its place when:
the subtype is genuinely a specialization and satisfies the substitution principle everywhere
the hierarchy is shallow, closed, and unlikely to grow combinations
you are modelling a closed set of variants — often better expressed with records and pattern matching in modern C#
a framework requires it (ControllerBase, DbContext, Exception)
The signal to watch is not "am I using inheritance" but "am I turning parts of my parent off". Protected flags, overrides that throw, and subclasses whose names contain And are all the same message: these are separate behaviours that were forced into one type.