mentis vulgaris
simple thoughts | jason smith
Is Your Code SOLID: The Dependency Inversion Principle
Posted by Jason Smith - 15/01/10 at 09:01:17 amThe mother of all SOLID principles. Nail this one, and you’ll keep your codebase supple — ready for just about any change you throw at it. The Dependency Inversion Principle comes in two flavors:
- HIGH LEVEL MODULES SHOULD NOT DEPEND UPON LOW LEVEL MODULES. BOTH SHOULD DEPEND UPON ABSTRACTIONS.
- ABSTRACTIONS SHOULD NOT DEPEND UPON DETAILS. DETAILS SHOULD DEPEND UPON ABSTRACTIONS.
An example should make this clear.
Consider a very simple requirement: read a serial port for ASCII characters, find any appropriate stock symbol and associated data and copy that data onto a TCP/IP socket (don’t laugh – those were real requirements: a trading partner’s trade desk wanted a connection to a former employer’s trading platform, and the exchange had no more ports to grant).
The MacGyver’ed up solution ended up with a dependency graph that looked like this:
More abstractly, this program reads characters from a source (the serial port), does a lookup, and then writes textual data to a destination (a socket). A reasonable one-off, but ultimately a problem. Because the high level module (symbol lookup) depended on low level modules (an RS232 serial port reader and a socket), we could only use it in an environment with an RS232 port and a socket. Any other use would require so much refactoring a rewrite actually started to make sense. It was a very simple hack … err … design that became very expensive to maintain.
Instead, if the symbol lookup module depended on a simple text reader abstraction, which the RS232 Serial Port reader then implemented, and the socket implemented a simple text writer abstraction, we suddenly open all the modules up to a world of other uses and environments:
With this change, all the classes follow the OCP. The behavior of the program is changed by replacing the RS232 Serial Port Reader with some other character reader (i.e., and extension) – no other classes need to change. As long as our line-of-business module (symbol lookup) depends on abstractions (TextReader and TextWriter), the data will flow no matter where it comes from, or goes to.
Usually, DIP isn’t violated in such an over-the-top way as that. It’s often much more subtle:
namespace Numa.Infrastructure.Client
{
public interface IUIHost
{
//...
/// <summary>
/// Get or Set Environment Configuration Setting
/// </summary>
Dictionary<string, string> EnvConfig
{
get;
set;
}
}
}
Note the implementation detail: IUIHost.EnvConfig returns a Dictionary<string,string>. Not only does the interface depend on that detail, it forces all of its consumers dependency on it simply by making it part of the interface. A reasonable SOLID refactoring of this interface would probably replace the Dictionary<string , string> class with an IDictionary<string , string> interface.
This simple change does two things to improve the quality of our design:
- It protects the clients of our abstractions from changes in our implementation of the abstraction. A derived design might deal with huge result sets, and so it loads the data lazily. A specific Dictionary type can’t do that, but something implemented under the IDictionary interface can.
- It documents a contract between our abstraction and clients of our abstraction – constraining both our designs, and the valid uses of our designs. The change says our interface returns an object that can map a string to another string.
Before we think we’re done, consider this: both the original and the changed code also says the IUIHost.EnvConfig property supplies a type where:
- String mappings can be added
- String mappings can be removed
- String mappings can be cleared
In other words, this interface promises to support those additional features. If we don’t want to make all of those promises, we should choose an abstraction that better describes what we mean (i.e., If we don’t want to allow modifications to the result set, we might consider an IEnumerable or ICollection return type)
Another place we see the DIP violated is in code written from a procedural perspective – code that depends on specific function calls instead of calling through abstractions or interfaces. They are often implemented in types with names like “FooManager” or “FooHelper”. For example:
using System;
using System.Security.Principal;
public static class AuthorizationService
{
public static bool HasPermission(
string action,
IIdentity identity)
{
// something
}
}
public class MyClass
{
public void DoSomething()
{
bool permitted = AuthorizationService.HasPermission(
"MyClass.DoSomething",
System.Threading.Thread.CurrentPrincipal.Identity);
if (permitted)
{
// blah
}
}
}
MyClass.DoSomething depends on the following implementation details:
- where the Identity comes from (the Thread’s CurrentPrincipal)
- it depends on where it gets the AuthorizationService from (it’s a global)
- the specific implementation of the AuthorizationService.
To understand this code’s resistance to change, try writing a unit test around MyClass.DoSomething() that only invokes a test stub AuthorizationService. It simply can’t be done.
AuthorizationService exposes the detail that there’s only one (implied by the fact that it’s a static class) callers will also depend on that detail, and that its services will always be accessible via a reference to the class name. In other words, it simply can’t be changed without forcing a rebuild of all clients.
An improved design might look something like:
using System;
using System.Security.Principal;
public interface IAuthorizationService
{
bool HasPermission(string action, IIdentity identity);
}
public class MyClass
{
IIdentity _user;
IAuthorizationService _authSvc;
public MyClass(IAuthorizationService authService,
IIdentity user)
{
_authSvc = authService;
_user = user;
}
public void DoSomething()
{
bool permitted = _authSvc.HasPermission(
"MyClass.DoSomething", _user);
if (permitted)
{
// blah
}
}
}
This technique, called Constructor Dependency Injection, breaks MyClass’s dependency on the detail that an AuthorizationService is implemented a particular way, and that it’s supplied in a particular way. By breaking that dependency, our designs become much easier to change and test.
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 …
Powered by WordPress with GimpStyle Theme design by Horacio Bella.
Entries and comments feeds.
Valid XHTML and CSS.