Friday, May 2, 2014

"Region Hell" in the Codebase

Do you have "Region Hell" in your codebase?

Do you understand what I mean by "Region Hell".  No?  Ok, read on...

I'm sure this has been written about before many times.  However I still makes me itch so I want to write about it.

Every developer starts out in life using Regions in code, so did I 

We start out early in life thinking man if I use Regions, how easy will it be to navigate sections of code in this class!  Hooray!  This is efficient.

But over much time good practices have developed in Software that have stuck and simply work.  When you get to know those principals you start to see code in a whole new light, and better light.

Clean code has become an expectation as a professional developer on teams that care.  

People like Martin Fowler, Roy Oshrove, Uncle Bob Martin an a slew of others have come up with tried and true development practices in design that keep code simple, readable, and maintainable.  

What they have come up with in terms of clean code and design is generally hard to argue and easy to see that it has huge impact on your teams and companies and dramatic change toward the better if you as a developer start caring about them.  This is indeed part of Software Craftsmanship.  My definition of Software Craftsmanship is a specific set of principals I follow...most can be derived from Uncle Bob Martin, and 8th light.

For developers who are less experienced, or who have simply not seen much code other than the code they have worked on because they might have been with a company for a long, long time, regions seem to be a requirement and how dare you say not to use them right?  Nothing against you but please, please read on here.

Again I used to use regions.  But then I came to realize after learning about practices and patterns that they are a huge code smell.  And when you start seeing regions nested such as in methods, then you really should run for the hills.

Why Regions Are Code Smell


First and foremost as a basis for understanding why,  I highly suggest you start learning about S.O.L.I.D.  These are practices good development shops live by and every developer should be constantly striving to learn and apply to code.  They look for this kinda stuff when they pair program, when they do formal code reviews, etc.  You'll find me talk a lot about it in my blog posts or while working with me as I code or pair.

So please do yourself a huge favor and start learning about it.  There are great videos all over the net.  Here are a few that I recommend, and well worth the few dollars.  Sometimes people, it's worth paying for some stuff.  

Let me get this out of the way first because you're gonna hear me talk about Uncle Bob and 8th light a LOT.  

Yea Yea, Dave there you go again talking about Uncle Bob.  Yep, get used to it, because I'm probably not gonna stop.

The reason I preach Uncle Bob Martin so much is for a lot of reasons but one of them is he teaches and passes on his experience to our trade in a very well communicated and easy to understand, yet entertaining fashion.  Don't take my word for it, try one of his videos and you will see.  I have actually asked them to make the "Clean Code" video free and they're considering doing that so I can help spread the word about these videos, because I really want to spread the word about them because they are effective as a teaching tool.  I kinda look at them like Khan Academy, they are very effective.  But again, there are also all sorts of good vids out there  on SOLID from people from Google, you name it...not just from Uncle Bob.

The first principal of S.O.L.I.D is "Single Responsibility".  Most people will explain it as

"A class should have one, and only one, reason to change."  

But honestly IMO, that initial sentence tells anyone new to this principal absolutely nothing at first glance.

I like how it's explained in the book Clean Code:

"Each Responsibility in a class is an axis of change.  When the requirements change, that change will be manifest through a change in responsibility amongst the classes.  If a class assumes more than one responsibility, then there will be more than one reason for it to change.

If a class has more than one responsibility, then the responsibilities become coupled.  Changes to one resonsibility may impair or inhibit the class' ability to meet the others.  This kind of coupling leads to fragile design that break in unexpected ways when changed"

I can summarize it like this:

If your class is doing too much and too many things, you end up with long ass classes.  A class should only be doing ONE thing.  Just as "a" method should only be doing ONE thing which is another topic and that also is a sign of code smell when your methods are long.

So why does this apply to regions?  

Well once you start getting to know and applying the Single Responsibility principal to code, you'll start to look at regions as "ew, yuck, why do we have all these regions in this class" because that's a huge red obvious flag that this class is doing way, way too much.  You won't realize this if all you've ever known is regions so know that and be open to change.

You shouldn't have to scroll endlessly down a class, which then makes you think oh, I should now add regions to a class.  If you do you're not designing well.  Be open to thinking differently about this.

The point is, my class should not be so long as to start thinking I need to group stuff into regions just to make it more readable.  And no this has nothing to do with "over-engineering".  I know I'll hear that from a few people and please, stop with that cop-out.  I've seen over-engineered systems and getting rid of regions and splitting stuff out is common sense.

It's simple:  Tons of regions is almost always a sign of bad design.  In fact that's not design at all, that's a mess when you have a ton of regions. 

You should never have to use regions, or lets say rarely.  I admit I'm using regions only for our TDD tests only because we haven't come up with a better way to organize and maintain our Unit Test classes, but we'll change all that soon.  Otherwise seeing regions all over the place really stinks.

For Readability

Who wants to sift through tons of lines of code in a single class anyway?  Sure, you can use the nice little dropdown at the top corner of Visual Studio to see all the methods in x class but then ask yourself why the heck do you even have to do that!  The class again shouldn't be this long to begin with, that's insane!

For Maintainability

By adhering to SOLID, seeing code split out into specific responsibilities will clear the forest. Trust me, again you need to see it to believe it.  If you have lived your life in long classes, you need to see it and work with it to see the light.

Ok Then What do you Suggest Dave?

Like many who follow SOLID, I suggest the following:

  • start refactoring.  Slowly refactor classes in your codebase (without breaking behavior) and breaking up classes into several smaller classes over time  
          "Did you just say a bunch of smaller classes Dave?"  Hell yes I did.  "Well wouldn't that make it harder to maintain if there are now a bunch of small classes, whew, I like that I can just navigate one huge class that has 300+ lines, even thousands".  Really?  You love to scroll up and down, expand/unexpand regions of hidden long coupled code?  Well have you ever worked with a bunch of small classes?  I bet you would change your mind if you saw this and experienced working in it.  And also it's nice to buy and Install Resharper to make things even quicker to navigate as a whole within Visual Studio period and you will stop thinking it's hard to look and get to things.

  • Remove the regions in the original class  and do not add them to the new classes you are splitting that 1000 line class out into.  The new classes you create should be small enough as to not need regions!  That's the whole point of single responsibility.  If you are adhering to it, most cases you'll find the class to be small enough that you either don't have to scroll or scroll a little bit
  • Stop using regions period going forward, instead look to apply the Boy Scout Rule as you code
    • Boy Scout Rule for code was originally conceptualized in Uncle Bob's Clean Code book which says keep the campground cleaner than you found it.  That could be several things:
      • renaming methods, variables, classes to make it more readable
      • splitting methods out into several for code reuse or "DRY"
      • removing regions after you split out a class that is violating Single Responsibility
      • etc.
Open your mind up to the fact that yes, what seems to be working for you, might actually be a mess and a horrible way to code and you could be going faster after splitting out classes.  Splitting them out may seem like a lot of work to you but it's actually expected of you as a good Software Engineer if you are trying to adhere to SOLID and care that your teammates have to also work in and support the code YOU write and HOW you write it.  

Don't be a "Clean Code Hater"

Don't let yourself fall into this trap.  Stop being a cowboy developer, and please stop saying it's only for "perfectionists".  Look yourself in the eye and think again why you are a Software Engineer.  Stop being lazy.  As harsh as that may sound (I'm actually a really nice guy though at work), it's honestly true.  We need devs in our profession who take code seriously.

The codebase is not yours, it's everyone's.  Treat it with care.  And care about clean code.  This is not "perfectionism", for those who like to knock down principals, it's called "professionalism".

You may not see the light yet, but you will.  Again start reading about SOLID and find fellow devs who already have code that is fairly clean and that will also help you start to see possibly.  

Get off the "Code & Run Bandwagon" and find a developer who already understands and has applied SOLID every day and have them mentor you.  Pair with those kind of devs and you'll'll be a great moment in your career to start learning about these principals.

We are all here to learn together as Software Engineers in a tactful manor.  Part of learning means refactoring messes and acknowledging that maybe the code you're so used to could be drastically improved for the team, and for yourself, and obviously positively affecting the business.  

Refactoring is goodness.  It's not something you should say never to do, or say people aren't supposed to be doing.  Instead you should be adding Unit Tests to cover your back as well and refactor!  Code Rot is not what our profession needs, we need to stop the Code Rot people.

No comments:

Post a Comment