BT

Becoming SOLID in C#

by Abel Avram on May 23, 2014 |

Brannon B. King, a software developer working for Autonomous Solutions Inc., has published an article entitled Dangers of Violating SOLID Principles in C# in MSDN Magazine, May 2014. The author outlines some of the mistakes developers can make in their C# code, breaking the SOLID principles and leading to code that is more difficult to extend or maintain.

King provides counter code samples and advice for all SOLID principles, but for the sake of brevity we have extracted just a few of them related to the Open-Closed Principle (OCP). OCP states that "software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification". According to King the following piece of code breaks OCP

void DrawNerd(Nerd nerd) {
   if (nerd.IsSelected) DrawEllipseAroundNerd(nerd.Position, nerd.Radius);
   if (nerd.Image != null) DrawImageOfNerd(nerd.Image, nerd.Position, nerd.Heading);
   if (nerd is IHasBelt) // a rare occurrence
      DrawBelt(((IHasBelt)nerd).Belt);
   // Etc.
}

because “you’ll have to modify this method every time a customer needs new things displayed—and they always need new things displayed,”  proposing instead a general procedure for drawing:

readonly IList<IRenderer> _renderers = new List<IRenderer>();
void Draw(Nerd nerd) {
   foreach (var renderer in _renderers)
    renderer.DrawIfPossible(_context, nerd); }

The idea is to

… write drawing classes (or classes about drawing classes) that implement a well-known interface. The renderer must have the smarts to determine if it can or should draw anything based on its input. For example, the belt-drawing code can move to its own “belt renderer” that checks for the interface and proceeds if necessary.

Another example breaking OCP is that of a class referencing an inheritor class

class Nerd {
public void DanceTheDisco() {
if (this is ChildOfNerd)
throw new CoordinationException("Can't");
   ...
}

}
class ChildOfNerd : Nerd { ... }

the author’s advice being that “base classes should never directly reference their inheritors.” This problem can appear in peer classes as well:

class NerdsInAnArc {
public bool Intersects(NerdsInAnLine line) {
    ...
   }
   ...
}

King explains:

Arcs and lines are typically peers in the object hierarchy. They shouldn’t know any non-inherited intimate details about each other, as those details are often needed for optimal intersection algorithms. Keep yourself free to modify one without having to change the other. This again brings up a single-responsibility violation. Are you storing arcs or analyzing them? Put analysis operations in their own utility class.

While strictly applying SOLID principles may not be necessary for small projects, their importance becomes more obvious for larger codebases where avoiding “spaghetti” code is quite desirable.

Hello stranger!

You need to Register an InfoQ account or or login to post comments. But there's so much more behind being registered.

Get the most out of the InfoQ experience.

Tell us what you think

Allowed html: a,b,br,blockquote,i,li,pre,u,ul,p

Email me replies to any of my messages in this thread

More Extremist Garbage by Jonathan Allen

SOLID takes reasonably design principals and pushes them to their absurd limits.

If you actually adhere to OCP, specifically the "closed for modification" rule, then every time you want to add a new method you have to create a new subclass. It isn't hard to see how quickly this can get out of control.

Then again, if you adhere to SRP as well then you'll need to break up System.Boolean into half a dozen different classes, each with one or two functions each. So I guess you won’t be adding any new methods in the first place.

And then there is ISP. If you follow SRP, then ISP is basically a no-op because there is only one responsibility and thus nothing to segregate.

Looping back to OCP, the open part is rather difficult to follow when value types cannot be inherited. Even System.String is not open for extension for very good reasons. Putting virtual methods on a class isn’t free; you take a performance hit from not being able to inline functions when the JIT runs.

The only thing SOLID has going for it is that it has a cool acronym. If you want something that will help you write code that is actually solid, look to the .NET Framework Design Guidelines.

Re: More Extremist Garbage by Abel Avram

Yes, anything taken to extremes is counter-productive. Look at the Agile movement with people struggling to share a keyboard when it feels awkward to them. For some it works, for some it doesn't.
There are no rules or principles that can guarantee one's arrival safely to the destination. It takes trial and error sometimes, it takes refactoring sometimes, and it always takes a lot of experience to make sure things are done right.
I agree with most of what you said, but I see some value in SOLID too.

Re: Extremist garbage? -- Disagree! by Michael Kramer

I disagree!
I must agree that extremism is a bad idea - but the SOLID principles are to me not extreme in any way. Contrary - I've seen all too much hard-to-maintain code being hard to maintain because it did not follow the SOLID principles.

I like my code to be DRY, Clean, and SOLID! To me these principles are guidelines that make code easier to develop, understand, extend, and, maintain. Design principles are like design patterns => They are helpful guidelines, not universal laws.

EXAMPLE: Liskov's Substitution Principle.
If you twist semantics when you extend a method in a descendant class you make it close to impossible to reason about the generic code involving the ancestor class because whenever your descendant executes the semantics are all different.

EXAMPLE: Interface Segregation Principle.
Agreed that ISP could be described as a subset of SRP but it is quite nice to have this important design principle called out by itself. Whenever I design an interface I ask myself:
Q1: Did I include any superfluous methods?
Q2: Does this interface actually serve multiple purposes?

EXAMPLE: Open Closed Principle.
This principle doesn't say you must inherit whenever you add functionality - instead it states that it is fully OK to EXTEND a class! But whenever you CHANGE existing behavior you actually force all dependent code to be retested - and possibly force any end-user to be retrained because UX behavior changed. That's why "closed for modification" is so dangerous to break.

Re: More Extremist Garbage by Vince Pan

I don't have

Re: More Extremist Garbage by Jonathan Allen

The alternative is the patterns found in the .NET Framework Design Guidelines.

There you won't find rules like "never modify a class once it has been shipped", the closed part of the Open/Closed Principal, except where it would actually create a breaking change.

You won't see SRP in it either. Look hard and it will probably warn against "god objects", but they don't go the extreme route that says even System.Boolean is too big.

Re: More Extremist Garbage by Jonathan Allen

The alternative is the patterns found in the .NET Framework Design Guidelines.

There you won't find rules like "never modify a class once it has been shipped", the closed part of the Open/Closed Principal, except where it would actually create a breaking change.

You won't see SRP in it either. Look hard and it will probably warn against "god objects", but they don't go the extreme route that says even System.Boolean is too big.

Re: More Extremist Garbage by Jonathan Allen

Without SOLID principles you're going to end up writing monster functions which know and do too much.


This is a perfect example of why I don't like how SOLID is being promoted. There is absolutely nothing of the SOLID guidelines that says anything about the content of individual functions. (Though my friend worked at a company where they took SRP to literally mean that you have one function per class. At which point then... well SOLID still doesn't say anything about what goes in that one function.)

Re: Extremist garbage? -- Disagree! by Jonathan Allen

This principle doesn't say you must inherit whenever you add functionality - instead it states that it is fully OK to EXTEND a class! But whenever you CHANGE existing behavior you actually force all dependent code to be retested - and possibly force any end-user to be retrained because UX behavior changed. That's why "closed for modification" is so dangerous to break.


That's not what the Closed part of the Open/Closed principal says. That's what EVERYBODY says about maintaining backwards compatibility.

If you look at what Meyer actually wrote, or the Wikipedia summary, you'll see that he's quite specific on this point:



Bertrand Meyer is generally credited as having originated the term open/closed principle,[2] which appeared in his 1988 book Object Oriented Software Construction. The idea was that once completed, the implementation of a class could only be modified to correct errors; new or changed features would require that a different class be created. That class could reuse coding from the original class through inheritance. The derived subclass might or might not have the same interface as the original class.




The principal known as "favor composition over inheritance" is in many ways a refutation of the Open/Closed Principal.

Re: More Extremist Garbage by Vince Pan

Have you considered the fact that your friend and the company he worked for interpreted SRP incorrectly?

It's not about one function per class, it's about responsibility and it's about keeping disparate business logic separate from each other. A class should have one reason and one reason only to change. That one reason might be order total and related calculations.

People don't just jump on bandwagons unless it makes sense to them :-)

Re: More Extremist Garbage by Luís Gabriel Nascimento Simas

Well. Using SOLID with DDD... excellent!

Allowed html: a,b,br,blockquote,i,li,pre,u,ul,p

Email me replies to any of my messages in this thread

Allowed html: a,b,br,blockquote,i,li,pre,u,ul,p

Email me replies to any of my messages in this thread

10 Discuss

Educational Content

General Feedback
Bugs
Advertising
Editorial
InfoQ.com and all content copyright © 2006-2014 C4Media Inc. InfoQ.com hosted at Contegix, the best ISP we've ever worked with.
Privacy policy
BT