mentis vulgaris
simple thoughts | jason smith
Is Your Code SOLID: The Interface Segregation Principle
Posted by Jason Smith - 12/01/10 at 07:01:02 pmSimplicity is the ultimate sophistication. ~ Leonardo da Vinci
Robert Martin, in his 1996 article says:
When clients are forced to depend upon interfaces that they don’t use, then those clients are subject to changes to those interfaces. This results in an inadvertent coupling between all the clients. Said another way, when a client depends upon a class that contains interfaces that the client does not use, but that other clients do use, then that client will be affected by the changes that those other clients force upon the class. We would like to avoid such couplings where possible, and so we want to separate the interfaces where possible.
An example of this problem can be seen in the following interface:
/// An interface that converts IIdentities into an IMyIdentity (i.e., an identity with Roles and properties)
public interface IAuthorizationProvider
{
IMyIdentity GetRolesFor(IIdentity identity);
MembershipInfo GetRoleMembershipInfo(string role);
IEnumerable<string> Roles { get; }
void AddRole(RoleInfo roleInfo);
void AddSubjectToRole(string role, IIdentity identity);
}
No one would argue that an interface with 5 methods isn’t manageable. But as we look at the code from a client perspective, it’s flabby. Reading through the body of code this sample was lifted from, there are two consumers of this interface:
- Users who are simply interested in determining whether an Identity is authorized to do something.
- Someone who needs to manage authorizations/role memberships, and what roles are associated with an identity.
The most common user is only interested in reading whether an identity is in a role. For that user, the rest of the interface is completely unnecessary, and unnecessarily coupled (to say nothing of the security risk having authorization management api’s available to the average user).
The ISP driven improvement for this design might look something like:
/// An interface that converts IIdentities into an IMyIdentity (i.e., an identity with Roles and properties)
public interface IAuthorizationProvider
{
IMyIdentity GetAuthorizations(IIdentity identity);
}
/// An interface that converts IIdentities into an IMyIdentity (i.e., an identity with Roles and properties)
public interface IRoleManager
{
MembershipInfo GetRoleMembershipInfo(string role);
IEnumerable<string> Roles { get; }
void AddRole(RoleInfo roleInfo);
void AddSubjectToRole(string role, IIdentity identity);
}
If you’re wondering why I didn’t derive IRoleManager from IAuthorizationProvider, consider this: if we’re going to all this effort to reduce coupling, then inheritance is the last thing we want. Inheritance is the strongest form of coupling you can have between two types (outside of C++’s concept of “friend”). Interface segregation is about making the surface area of our dependencies smaller. Because inheritance exposes derived classes to protected data in bases, by definition, we increase that dependency surface.
Keep your interfaces demure. You’ll sleep better. I promise.
Is Your Code SOLID: The Liskov Substitution Principal
Posted by Jason Smith - 20/12/09 at 05:12:57 pmIf you think of standardization as the best that you know today, but which is to be improved tomorrow; you get somewhere. ~ Henry Ford
I honestly think the lowly USB was one of the best inventions for Personal Computers. This one adapter allows you to connect your PC to your keyboard, mouse, cell phone, external hard drive, graphics tablet, MP3 Player or a soft drink chiller. So long as the device follows the appropriate USB standard, communication between the device and PC should simply work, no matter what device you use.
The Liskov Substitution Principal (named for Barbara Liskov, who first formalized it in 1987) works the same way – some objects define “plug types” or “protocols” (abstractions, interfaces and contracts) while others implement plugs that fit those plug types/protocols to do stuff. Clients implement sockets that fit those plugs, calling on those implementations to do their thing. So long as the implementation abides by the abstraction/interface/contract, the system will work, and the client really doesn’t need to care how.
For example, consider a simplified drawing API:
void Draw(IEnumerable<IShape> shapes, IGraphics destination)
{
destination.Pen = new Pen( …);
foreach( IShape s in shapes)
{
s.DrawOn(destination);
}
}
So long as a type implements IGraphics, it can be used as the “destination” parameter. The Draw method doesn’t care if the implementing type is a window, a printer, or a .PNG file writer.
Of course, things aren’t just that simple.
The protocol isn’t just the “shape” of the plug (or name and parameters of the method). For example, a USB device cannot draw more power than provided over the USB interface, and the USB device cannot hope to get more data across the interface than defined by the standard. In the same vein, an LSP compliant interface cannot demand or expect more of the client than the original interface describes.
Consider an API that takes an IEnumerable
using System;
using System.Collections.Generic;
public interface IFoo
{
/// <summary>
/// Take an enumeration of strings, and Frobbles
/// </summary>
/// <param name="items"></param>
void Frobble(IEnumerable<string> items);
}
The following represents an implementation that violates the LSP
public class FooClass : IFoo
{
/// <summary>
/// Take an enumeration of strings, and Frobbles
/// </summary>
/// <param name="items"></param>
public void Frobble(IEnumerable<string> items)
{
List<string> l = items as List<string>;
if (l == null)
{
throw new ArgumentException("Items is invalid type");
}
// ...
}
}
The reason FooClass violates LSP is it cannot be used in all places an IFoo says it could be used. IFoo.Frobble’s use of IEnumerable<string> says it is passed a read only enumerable (iterable) entity (i.e., you cannot add elements to or remove elements from it). FooClass, for some reason, wants List behaviors as well (adding, removing, indexing, etc). This additional requirement means FooClass can’t be used in all those situations that IEnumerables were successfully being passed before.
The way you fix this depends on a couple of things:
- Is IFoo NOT a published interface?
- you want to expose a List requirement on all clients (e.g, Add/Remove items)
If both 1 and 2 are true, you might consider changing the IFoo interface so that Frobble takes a List
If either 1 or 2 is false, then FooClass.Frobble cannot be changed, and must respond without an error when it receives any type derived from IEnumerable
In other words, interfaces (and abstractions) make promises you expect others to keep, and informs clients what sort of service they can expect.
Every interface or abstraction is a promise. It’s a promise you will have to live up to. One that others will have to live up to. So, be careful about what you promise, and be sure you understand what others are promising for you.
Is Your Code SOLID? OCP and Fighting the Cost Of Change
Posted by Jason Smith - 03/12/09 at 05:12:41 pmIt is better to keep your mouth closed and let people think you are a fool than to open it and remove all doubt. – Depending on who you ask, Mark Twain or Abraham Lincoln
Is it a good use of time and money to periodically pull reviewed, tested and unchanged code out of source control so the team can re-review and re-test it?
Probably not. If the code hasn’t changed, we’ll quickly experience diminishing returns.
Then again, all software changes, so it’s a moot point.
Or is it?
What if we could structure our designs in such a way that we can add features, without changing existing code?
Enter the Open/Closed Principle (OCP): software should be open to extension, but closed for modification.
To understand how this principle works, lets start with code that violates this principle. We usually find this around dynamic casts, “switch/case” and chains of if/else if/else statements.
Consider the switch/case statement. Every time we add a new case, we must modify code in the switch. Same with if/else: every time we need a new else, the original source must be modified. If behavior in a function is dependent on a successful dynamic cast of a parameter (using the as or is keywords), you’ll have to add new code to handle any new types. All this new code called in all these places must be tested in all its variations.
Here’s an example:
public class Foo
{
private object myStuff;
public void Bar(OutputDevice outputDevice)
{
switch(outputDevice)
{
case OutputDevice.Window:
DrawOnWindow(myStuff);
break;
case OutputDevice.Printer:
PrintStuff(myStuff);
break;
default:
throw new NotImplementedException();
}
}
}
Assume we get a new feature request: users want to save their stuff to a file. With the code as it’s currently written, we must modify the original shipping code and add an OutputDevice.File case (and quite possibly a new enumeration value – potientially breaking any other code using the original enumeration). Additionally, we need to test Foo.Bar to make sure our changes didn’t break the old code, and because Foo.Bar changed, we need to test all the code that calls Foo.Bar, and we need to test the new rendering codes to make sure that works.
Way, way, too much work for a “simple” change.
An OCP solution for this will turn all these conditionals into abstractions:
public interface IRenderStuff
{
void Render(object data);
}
public class RenderableWindow : IRenderStuff
{
public void Render(object data)
{
// ...
}
}
public class PrintRenderer: IRenderStuff
{
public Printer Printer { get; set; }
public void Render(object data)
{
Printer.Setup();
Printer.Print(data.ToString());
Printer.Close();
}
}
public class PrintRenderer : IRenderStuff
{
public void Render(object data)
{
using (System.IO.TextWriter writer = CreateFileWriter())
{
writer.Write(data.ToString());
}
}
}
Now, Foo.Bar can look like:
public void Bar(IRenderStuff renderer)
{
renderer.Render(myStuff);
}
Foo.Bar is now open for extension, and closed for modification. This one adjustment insures Foo.Bar’s doesn’t have to change when we add a new renderer. If it doesn’t change, it won’t have a bug injected into it (if the only change is a new rendering destination), and no longer needs to go through testing of all the rendering cases.
In fact, Foo.Bar need only be tested against a one stub implementation of IRenderStuff to verify it is holding up its end of the contract. Of course, each IRenderStuff implementation needs testing, but we had to do that anyway.
Here’s the payoff: this change reduced our test overhead (read: ways this code could break) from
×
NumberOfRenderingMethods
×
NumberOfBodiesOfCodeUsingTheOutputDeviceSwitch
to
Our cost of change just went from an exponential increase to a linear one.
Now, consider class member variables. Whenever code reaches into a class to access a member directly, we cannot change how that class is implemented without affecting (and potentially breaking) all of that class’ users. Every bit of code referencing a member variable is open (or “exposed”) to modification. Any change to our implementation means a change to all of its consumers.
To limit this effect, we encapsulate the variables behind methods and properties, closing those relationships to modifications in the code.
It’s not much of a leap to realize any member variable access violates this principle. So we minimize the area affected by making member variables private (not protected, that’s a cheap cop-out). By doing this, only member functions of the defining class are exposed to changes in those variables. All other consumers are closed to those changes.
The same thing applies to global variables. In the same way non private data members violate the OCP, so do global variables. Every module that depends on a global variable can never be closed to modification with respect to that global.
The Single Responsibility Principle
Posted by Jason Smith - 25/11/09 at 10:11:25 pm“It is a bad plan that admits of no modification.”
— Publilius Syrus, First Century BC
The Single Responsibility Principle states a class, module or function has one responsibility.
Ultimately, this is the definition of cohesion.
Often this principle is defined as a module (or class, or function) should have only one reason to change. It assumes code that must be changed for a myriad of reasons is usually arbitrarily coupled code. A clue you have code violating this principle is when changing an implementation of something in one class, you find you need to make code changes in several classes.
For an example, consider the following:
public class DessertProvider : System.Configuration.Provider.ProviderBase
{
private string _connectionString;
public override void Initialize(string name, NameValueCollection config)
{
base.Initialize(name, config);
_connectionString = config["connection"];
}
public bool CanHaveDessert(string name, string tonightsDessert)
{
bool result = false;
using (SqlConnection cn = new SqlConnection(_connectionString))
{
cn.Open();
SqlCommand cmd = new SqlCommand(QueryString, cn);
string query = @"SELECT COUNT(*) FROM foods
INNER JOIN plan_foods on plan_foods.ID = foods.ID
INNER JOIN plans on plan_foods.Plan = plans.ID
INNER JOIN people on people.Plan = plans.ID
WHERE foods.Name = @1 and people.Name = @2";
// ... elided ... execute the query to see if
// tonightsDessert is in "names" diet plan
}
return result;
}
}
This code runs in the app that tells an individual staff person whether a client gets dessert.
There are (at least) 3 unrelated reasons this code may have to change:
- The algorithm that decides whether someone gets dessert is changed.
- The way the source data is persisted.
- The algorithm that configures the code to fetch the diet plans (via the ProviderBase)
To understand the consequences of that design, consider a new requirement. Some of our customers discovered most of their clients have identical diet plans. To improve the dietician staff workload, they’d like to manage diet plans for groups of people instead of modifying each person’s individual plan.
Being the customer focused organization we are, we work up a new design for our next release. Now, instead of a diet plan belonging to a user, we subscribe multiple people to a diet plan.
Unfortunately, that change just broke the desert approval module. This lack of cohesion means that a design change to improve the life of dieticians has totally hosed how we determine whether anyone gets dessert (I’m pretty sure it messed up the Salad Module, too). What other distantly related code has that change broken?
So – we search, fix, release to QA. Search and fix the one’s we missed the first go-round. Test. Our stakeholders wonder why the change is taking so long. More searching and fixing.
Version 3 — the SOA release! Meal plans will be exposed via an SOA Service instead of a database. So, we must find all the code using the database, and change it to use an SOA enabled interface. Unfortunately, the compiler won’t help us – despite the change in architecture, the code is remains syntactically valid. More searching, fixing, testing, late nights and explaining to stakeholders. Here’s hoping for good test coverage.
Hmm — we also no longer need the database connection information coming in through the ProviderBase. It’s dead code. So, let’s rip it out. Another QA drop. Or are we so late in the cycle, we decide to leave the dead code in (we’ll get it in a service pack … riiiiight)?
So, why did the database schema need to be exposed to the DessertProvider? Why did it have to know about a database at all, when all it wanted was a meal plan for a person? And what is it about dessert determination that requires configuration via the .NET Provider Model? The answer to both of those questions is “nothing”. The original design insists it does, and then gets in the way of the inevitable changes.
Here’s an alternative approach that better separates responsibilities:
///
/// Supplies the algorithm to determine whether a patient gets
/// dessert.
///
public class DessertProvider
{
private IMealPlanRepository _mealPlans;
public DessertProvider(IMealPlanRepository mealPlanRepository)
{
_mealPlans = mealPlanRepository;
}
public bool CanHaveDessert(string name, string aDessert)
{
IMealPlan plan = _mealPlans.ForMember(name);
return plan.Contains(aDessert);
}
}
///
/// Describes a protocol for getting at our MealPlans
///
public interface IMealPlanRepository {
IMealPlan ForMember(string memberName);
}
///
/// Implement the IMealPlans interface over an SQL Server database
///
public class SqlMealPlanRepository : IMealPlanRepository
{
public SqlMealPlanRepository(string connectionString)
{
// ...
}
public IMealPlan ForMember(string memberName)
{
// SQL query goodness here ...
}
}
/// Some entity that knows how to supply our various services.
/// Consider an IoC container (such as Unity or StructureMap)
public class MyAppFactory
{
// ...
public MyAppFactory(NameValuePair configData)
{
ConfigData = configData;
}
private NameValuePair ConfigData
{
get;
set;
}
public IMealPlans MealPlans
{
get
{
return new SqlMealPlanRepositor(
ConfigData["connectionstring"]);
}
}
}
Now – when the meal plan schema changes – the only code that needs to change is our SqlMealPlanRepository.
Nothing else. No hunt/fix/test/hunt/fix/test.
If we go SOA, the only code that needs to change is a new IMealPlanRepository implementation served up by our Factory.
That’s it.
If the algorithm to determine whether someone gets desert changes, we change our DessertProvider.
Period.
But wait – there’s more! As an added bonus – our code just became unit-testable (the original was testable in the large, but it wasn’t unit testable – it required a functioning SQL DB, with an appropriate schema, and loaded with data that the other tests can’t mess with). We can test the DessertProvider in isolation without having a database or SOA bus around.
And they thought giving you a flashlight with the Snuggie was a good deal …
Software is Softest When It’s SOLID and DRY
Posted by Jason Smith - 31/07/09 at 06:07:00 pmIt is not the strongest of the species that survives, nor the most intelligent, but the one most responsive to change. ~ Charles Darwin
The most frustrating software projects I’ve worked with all had one thing in common:
Continue reading Software is Softest When It’s SOLID and DRY…
If You’re Not Designing For Change, You’re Wrong.
Posted by Jason Smith - 31/07/09 at 06:07:39 pmThose who cannot remember the past are condemned to repeat it.
– George Santayana
For years, I defined “good software design” as something that produces robust software while satisfying or exceeding requirements. I discovered holding on to that mantra meant I would eventually starve.
Continue reading If You’re Not Designing For Change, You’re Wrong….
One Step Closer to Expertise
Posted by Jason Smith - 29/07/08 at 07:07:14 amAn expert is a person who has made all the mistakes that can be made in a very narrow field. — Niels Bohr
Early in my consulting career, I experienced losing a client in mid project. While I’d love to blame the client and some hideous set of working conditions, it was most definitely my mistake.
Continue reading One Step Closer to Expertise…
You want to grow your business. Your software doesn’t.
Posted by Jason Smith - 24/07/08 at 02:07:20 pmMost businesses have big plans. Growth plans. Plans to increase volume, sell more products, carry fewer skus, carry more skus, reduce redundancies, improve processes.
Your software probably has different ideas.
Continue reading You want to grow your business. Your software doesn’t….
Powered by WordPress with GimpStyle Theme design by Horacio Bella.
Entries and comments feeds.
Valid XHTML and CSS.