Anatomy of a "Small" Software Design Change

File this one away for the next time your boss comes in and asks,lumberg[1]

Yeaaah, I’m going to need you to make that little change to the code. It’ll only take you a couple hours, right?

Software has this deceptive property in which some changes that seem quite big and challenging to the layman end up being quite trivial, while other changes that seem quite trivial, end up requiring a lot of thought, care, and work.

Often, little changes add up to a lot.

I’m going to walk through a change we made that seemed like a no-brainer but ended up having a lot of interesting consequences that were invisible to most onlookers.

The Setup

I’ll provide just enough context to understand the change. The project I work on, ASP.NET MVC allows you to call methods on a class via the URL.

For example, a request for /product/list might call a method on a class named ProductController with a method named List. Likewise a request for /product/index would call the method named Index. We call these “web callable” methods, Actions. Nothing new here.

There were a couple rules in place for this to happen:

  • The controller class must inherit from the Controller class.
  • The action method must be annotated with the [ControllerAction] attribute applied to it.

We received a lot of feedback on the requirement for having that attribute. There were a lot of good reasons to have it there, but there were also a lot of good reasons to remove it.

Removing that requirement should be pretty simple, right? Just find the line of code that checks for the existence of the attribute and change it to not check at all.

The Consequences

Ahhh, if only it were that easy my friend. There were many consequences to that change. The solutions to these consequences were mostly easy. The hard part was making sure we caught all of them. After all, you don’t know what you don’t know.

Base Classes

So we removed the check for the attribute and the first thing we noticed is “Hey! Now I can make a request for /product/gethashcode, Cool!

Not so cool. Since every object ultimately inherits from System.Object, every object has several public methods: ToString(), GetHashCode(), GetType(), and Equals(), and so on... In fact, our Controller class itself has a few public methods.

The solution here is conceptually easy, we only look at public methods on classes that derive from our Controller class. In other words, we ignore methods on Controller and on Object.

Controller Inheritance

One of the rationales for removing the attribute is that in general, there isn’t much of a reason to have a public method on a controller class that isn’t available from the web. But that isn’t always true. Let’s look at one situation. Suppose you have the following abstract base controller.

public abstract class CoolController : Controller
{
  public virtual void Smokes() {...}

  public virtual void Gambles() {...}

  public virtual void Drinks() {...}
}

It is soooo cool!

You might want to write a controller that uses the CoolController as its base class rather than Controller because CoolController does some cool stuff.

However, you don’t think smoking is cool at all. Too bad, Smokes() is a public method and thus an action, so it is callable. At this point, we realized we need a [NonAction] attribute we can apply to an action to say that even though it is public, it is not an action.

With this attribute, I can do this:

public class MyReallyCoolController : CoolController
{
  [NonAction]
  public override void Smokes()
  {
    throws new NotImplementedException();
  }
}

Now MyReallyCoolController doesn’t smoke, which is really cool.

Interfaces

Another issue that came up is interfaces. Suppose I implement an interface with public methods.  Should those methods by default be callable? A good example is IDisposable. If I implement that interface, suddenly I can call Dispose() via a request for /product/dispose.

Since we already implemented the [NonAction] attribute, we decided that yes, they are callable if you implicitly implement them because they are public methods on your class and you have a means to make them not callable.

We also decided that if you explicitly implement an interface, those methods would not be callable. That would be one way to implement an interface without making every method an action and would not require you to annotate every interface method.

Special Methods

Keep in mind that in C# and VB.NET, property setters and getters are nothing more than syntactic sugar. When compiled to IL, they end up being methods named get_PropertyName() and set_PropertyName(). The constructor is implemented as a method named .ctor(). When you have an indexer on a class, that gets compiled to get_Item().

I’m not saying it was hard to deal with this, but we did have to remember to consider this. We needed to get a list of methods on the controller that are methods in the “typical” sense and not in some funky compiler-generated or specially named sense.

Edge Cases

Now we started to get into various edge cases. For example, what if you inherit a base controller class, but use the new keyword on your action of the same name as an action on the base class? What if you have multiple overloads of the same method? And so on. I won’t bore you with all the details. The point is, it was interesting to see all these consequences bubble up for such a simple change.

Is This Really Going To Help You With Your Boss?

Who am I kidding here? Of course not. :) Well..., maybe.

If your boss is technical, it may be a nice reminder that software is often like an iceberg. It is easy to see the 10% of work necessary, but the other 90% of work doesn’t become apparent till you dig deeper.

If your boss is not technical, we may well be speaking a different language here. I need to find an analogy that a business manager would understand. A situation in their world in which something that seems simple on the surface ends up being a lot of work in actuality. If you have such examples, be a pal and post them in the comments. The goal here is to find common ground and a shared language for describing the realities of software to non-software developers.

What others have said

Requesting Gravatar... Haacked Apr 24, 2008 5:47 PM
# re: Anatomy of a Design Change
Note that in the latest MVC code, we changed what an action looks like again. But being up to date with our latest code is not the point of this post.
Requesting Gravatar... Gary Sherman Apr 24, 2008 6:20 PM
# re: Anatomy of a "Small" Software Design Change
It’ll only take you a couple hours, right?

"only" is a 4 letter word.

http://www.37signals.com/svn/posts/439-four-letter-words
Requesting Gravatar... Chad Myers Apr 24, 2008 6:35 PM
# re: Anatomy of a "Small" Software Design Change
Good post, Phil. 37signals had a post kinda like this about the 'four letter words' of collaboration:
http://www.37signals.com/svn/posts/439-four-letter-words

(credit to Gary Sherman @ Dovetail for finding it)
Requesting Gravatar... GlenH Apr 24, 2008 6:46 PM
# re: Anatomy of a "Small" Software Design Change
Whew! After reading this, I'm thinking you should go back to the original paradigm of requiring a [ControllerAction] attribute. Seems like it would be easier to apply an attribute exactly where you want it vs. having to remember all the rules to apply to prevent something you DON'T want to happen.
Requesting Gravatar... Sean Chambers Apr 24, 2008 7:02 PM
# re: Anatomy of a "Small" Software Design Change
When it comes to Controllers this is something that MonoRail has struggled with very frequently. Once you start using inheritance in Controllers you expose all of said ugliness that you dont want in derived controllers.

What MonoRail has done to mitigate this problem is introduce DynamicActions. It is a lot easier to use associations over inheritance when it comes to Controllers and some form of reusable actions should be introduced that can be plugged in or not without having to add extra code to not use an action. When designing controllers I rarely use inheritance as it almost always doesn't fit well. Sometimes it does work, however this is the exception.

I dont think I like the idea of introducing attributes to simply hide actions however, this adds extra noise that isn't as much as [ControllerAction], but noise all the same.

You must really dislike people like me by now, but I respect honesty and I try to give the same to everyone =) Just my 2 cents!
Requesting Gravatar... DM Apr 24, 2008 7:47 PM
# re: Anatomy of a "Small" Software Design Change
Ummm... To be honest, I don't fully understand this problem, so I may be way off here...

Doesn't this seem like a security problem waiting to happen? You really want to make public methods on the controller class callable by default? Through the web?
Requesting Gravatar... Haacked Apr 24, 2008 7:56 PM
# re: Anatomy of a "Small" Software Design Change
@Sean I disagree with the noise claim. It's only "noise" if you use it. And when you need it, you're glad it's there.
Requesting Gravatar... Jeff Apr 24, 2008 8:59 PM
# re: Anatomy of a "Small" Software Design Change
The obvious solution here is to mark what is acceptable as a web callable method, instead of trying to mark what isn't. Less chance of errors that way.
Requesting Gravatar... Lou D Apr 24, 2008 9:20 PM
# re: Anatomy of a "Small" Software Design Change
I feel your pain and it sounds like there's a good handle on what's come up. In the end you get to think about these issues so the rest of the world doesn't have to. So thanks for that. :)

I know it wasn't really the point of your post, but I have to say I agree with the decision to omit the ControllerAction attribute from a DRY standpoint. I've made a controller, I've declared a method public, and 99% of the time an explicit attribute would be an "Are you sure?" prompt.

Maybe the [NonAction] could be blended away into a restful-allowed-method attribute... As in [Accept(Get|Post|None)]?

Requesting Gravatar... Eric Hexter Apr 24, 2008 9:27 PM
# re: Anatomy of a "Small" Software Design Change
oh you had me at Lumberg. I appriciate the team making this change and dealing with all the unknowns that came with it!
Requesting Gravatar... Vijay Santhanam Apr 24, 2008 9:38 PM
# re: Anatomy of a "Small" Software Design Change
Good story. So true.

Fortunately, cos of testability, things swung round in your favor.
Global warming shrunk the iceberg?
Requesting Gravatar... Haacked Apr 24, 2008 9:57 PM
# re: Anatomy of a "Small" Software Design Change
@Lou the idea is that [NonAction] makes it so that the action doesn't even exist. Whereas what you proposed would mean that the action does exist, but returns a 405 "Method not allowed". They are semantically different.

I suppose, could remove the NonAction attribute and you could make a public method "not an action" by simply calling HandleUnknownAction.
Requesting Gravatar... Haacked Apr 24, 2008 10:06 PM
# re: Anatomy of a "Small" Software Design Change
@DM the part you missed because I left it out is that there's pretty much no good reason to have a public method on a controller class be public unless you intend for it to be web callable. In other words, you opt in to this web callable behavior by inheriting from our Controller class and by hosting your class in our framework. So these things aren't web callable by accident.

And since every method in a controller is generally an action method and intended to be web callable, it's pretty repetitive to have to mark each one as web callable. You already told us you wanted these things to be callable by way of using our web framework and inheriting from Controller.

This addresses @Jeff's question too. The reason we still need the [NonAction] attribute (or equivalent) is there are ways in which a method might be public due to inheritance that you wish to hide.

For example, an abstract base controller for your company might have common actions and you're supposed to use it. But you want to hide one of those actions.
Requesting Gravatar... Jarod Apr 24, 2008 11:35 PM
# re: Anatomy of a "Small" Software Design Change
"At this point, we realized we need a [NonAction] attribute we can apply to an action to say that even though it is public, it is not an action."

So is that better than having [Action] in the first place? Seems like we went backwards...

"Another issue that came up is interfaces. Suppose I implement an interface with public methods. Should those methods by default be callable? A good example is IDisposable. If I implement that interface, suddenly I can call Dispose() via a request for /product/dispose."

See previous point :)
Requesting Gravatar... Benny Thomas Apr 25, 2008 1:03 AM
# re: Anatomy of a "Small" Software Design Change
This sounds like a stupid idè, but shouldn't ControllerActions return a ActionResult. So only methods that return ActionResult would be accepted as ControllerActions. Inheritance on Controllers requires the Class to implement the Actions it want's to expose from it's base.

The implementation only needs to redirect the action to the base. This seems like a troublesome solution, but the gain is that it is easy to see which Actions a Controller exposes to the world.

My 2 cents.
Requesting Gravatar... Vijay Santhanam Apr 25, 2008 1:16 AM
# re: Anatomy of a "Small" Software Design Change
I can imagine this must've been an annoyance before the ActionResult came along.

Out of curiosity, how much time was allocated for the switch to public web-callable? how much time did that blow out to?

Really good example btw.
Requesting Gravatar... Richard Apr 25, 2008 3:09 AM
# re: Anatomy of a "Small" Software Design Change
I understand the "methods in a controller are implicitly actions" line of reasoning, but IMHO the potential for hard to spot problems related to interface inheritence (e.g. with your IDisposable example) mean a "safe by default" policy would require explicitly marking which methods are actions rather than which are not.
Requesting Gravatar... Mike Apr 25, 2008 5:49 AM
# re: Anatomy of a "Small" Software Design Change
What was the great argument for ditching the attribute?

The whole dry thing really gets me as it's not really dry, the config is jushed pushed of to another location, like the POCO folks that hack out xml mapping files.

IMHO using [NoAction] is pure insanity.
Requesting Gravatar... Offering A Solution Apr 25, 2008 6:00 AM
# re: Anatomy of a "Small" Software Design Change
It's interesting that no one has offered up a real-world analogy. I'll give it a shot. Most guys, and some gals, will understand on a basic level, the concept of working on a car, right? Well, maybe.

I used to own a 1968 Land Cruiser -- nice rig. Had the bare-bones basics. Three on the tree, flat six, no stereo, manually locking hubs. I needed to replace the fuel pump once, so I popped open the hood, and there it was on the right side of the block. Took me less than 30 minutes to replace.

Then, years later, my dad needed his fuel filter replaced in his rig -- a 1998 (?) Jeep Cherokee Sport. Now, I wasn't there, but my uncle was. I was figuring, eh, it'll take them about 30-60 minutes to do the job. Problem was, solving the same problem on that rig was quite a bit different, as it turns out that the automobile industry had progressed quite a bit in thirty years. After looking in the engine compartment, searching around, and finally referencing the appropriate manual, they learned that the pump wasn't in the engine compartment at all, but was in the gas tank itself. So, "changing the fuel pump" (same problem on my '68 Cruiser) ended up being a very complicated task, indeed.

Perhaps not the best analogy, but it might be a start. Of course, I'm not sure that likening software types to mechanics is such a good idea... We all know how many people love mechanics.
Requesting Gravatar... Fredrik Normén Apr 25, 2008 6:40 AM
# re: Anatomy of a "Small" Software Design Change
I think everything should be locked from start, and then we unlock things that should be open for public. By open up everything by default require us to make sure we don’t forget to close the things that shouldn’t be open. It’s easy to unlock things that should be public than lock thinks that shouldn’t at least that is my opinion.

Btw, why should it be possible at all to make a call to a method that doesn’t return an ActionResult? I can’t see why that should be possible at all.
Requesting Gravatar... BrianE Apr 25, 2008 7:07 AM
# re: Anatomy of a "Small" Software Design Change
In my experience it is generally the edge cases that cause all the problems. Non-technical bosses tend to think in general terms so it is very hard for them to internalize all of the problems edge cases create.

To help them understand you need to find something they are interested in and relate it to that. If they are a skier, you might relate it to a bird landing on a particular patch of snow that causes and avalanche. For some one into home improvement you might relate it to removing a section of a wall to expand you kitchen only to find you have a termite infestation.
Requesting Gravatar... Haacked Apr 25, 2008 7:41 AM
# re: Anatomy of a "Small" Software Design Change
@Benny, you just described our current implemention. We now do have action methods return an ActionResult.
Requesting Gravatar... Al Pascual Apr 25, 2008 8:18 AM
# re: Anatomy of a "Small" Software Design Change
Great post as also sounds like you are complaining about your boss!
Requesting Gravatar... Jacob Apr 25, 2008 9:20 AM
# re: Anatomy of a "Small" Software Design Change
Here are some possibly useful analogies for things that seem easy to an outsider.

Accounting: changing the GL Account number for COGS.
Finance: stock buybacks.
Board of Directors: hiring a CEO/adding a new director.
C-level officer: hiring an executive assistant.
Sales: closing.
Operations: adding/changing a product line.
Marketing: adding/changing a product line.
Requesting Gravatar... Stephen Apr 25, 2008 9:21 AM
# re: Anatomy of a "Small" Software Design Change
While I understand the reason this was done, and I've often found I've been really proud at how clean something worked out, only to later realize that, while clean.. its actual goal didn't really make sense..

WCF is a good comparison to this in that WCF took the route to be explicit.. everything has to be explicitly marked to be callable.

Of course it isn't likely to change back, but personally I would of preferred an attribute to state what IS an action..

I spent a lot of time thinking about the MVC in comparison to WCF.. for example, what about controllers that were only defined by an attribute? no inheritance.. the controller currently actually holds a lot of the execution handling, and makes them a bit messy.. in what I see as a controller.. it should be related only to what you define...

WCF then lets you get context via static methods (much like how one would use HttpContext.Current).. they have OperationContext etc etc..

It just appeared to me that WCF has a lot in common here.. services vs controllers.. they are highly similar concepts.. and WCF is such a clean implementation I would of liked the MVC to follow..

However, I've never built an MVC, so you guys probably have a list of reasons why the MVC shouldn't be inspired by WCF..
Requesting Gravatar... JacobM Apr 25, 2008 9:32 AM
# re: Anatomy of a "Small" Software Design Change
I agree about the issue of edge cases -- sometimes I get requirements where there are only ever going to be two types of something. Ever. Except, well, maybe very occasionally there will be three types. Non-technical people don't seem to understand that we have to design architecture to support the limit of what may happen, not just the typical case.

So an example might be feeding a bunch of kids. We'll start with hot dogs. Most kids like hot dogs, right? But it turns out some of them are vegetarians. So the boss comes down and says, hey, let's also have peanut butter sandwiches -- that'll be easy to add. And it is -- except that it turns out that some kids are so allergic to peanuts that they can't even be at a table where peanut butter is being served. So now suddenly we have to query all the kids and their parents and divide up the group, and have two sets of utensils and so forth.
Requesting Gravatar... Benny Thomas Apr 25, 2008 10:43 AM
# re: Anatomy of a "Small" Software Design Change
@Haacked

I believed you only did the ActionResult on RenderView. But it's nice to hear that you did this for Actions to. If I could say it, I would say that this is a good design ;)

I'll look @ the solution, my wife needs a little shop, so I'll will try implement it with Asp.Net MVC.

Have a nice weekend.
Requesting Gravatar... engtech Apr 25, 2008 11:01 AM
# re: Anatomy of a "Small" Software Design Change
I try to put features into a holding pattern if they don't have an obvious benefit.

Just today I got to avoid work because we didn't close the customer we would have needed the feature for. I'm glad I waited a few days to start it :)
Requesting Gravatar... SuperJason Apr 25, 2008 1:54 PM
# re: Anatomy of a "Small" Software Design Change
I wrote an article on how to show your boss you're doing a good job:
http://www.ytechie.com/2008/04/how-does-your-boss-know-you-doing-great.html

I realize it's not related to MVC, but it discusses that common problem in software, where your boss just doesn't understand how good a job you're doing.
Requesting Gravatar... Lou D Apr 25, 2008 4:02 PM
# re: Anatomy of a "Small" Software Design Change
I have another analogy one for you that's also based on cars and personal. Scuffed paint and broken window.

I scuffed the front bumper on a parking thing, not bad but enough to show and it had some yellow paint highlights. I'm thinking it's just painting over it with a standard color. I asked for an estimate when I was getting some routine stuff done anyway and I'm thinking a hundred or two maybe on the outside. Nope! Seven or eight hundred with materials and labor because you also have to blend the paint with the surrounding area and it has an automotive finish and the small size really doesn't matter in the end.

On the other hand one day my window didn't go up. So it's halfway down and I'm thinking I don't even know where the fuses are to check that, and it could be the car's electrical is fried, or maybe the door will need to be torn down if it's a mechanical failure in the window itself. After the paint surprise I'm just hoping it's less than a thousand but the guy doesn't even need to look at it to say, "Probably the motor, they go all the time. It'll be eighty bucks and about an hour."

So - scuffed paint is disaster and broken window is nothing.
Requesting Gravatar... Chris Chandler Apr 25, 2008 8:45 PM
# re: Anatomy of a "Small" Software Design Change
Thanks for the great take on this.
Love the behavior, glad to see something at microsoft that has a stance. Most other teams would implement a switch between negative and positive attributes with a class level attribute, thus making my life more and more configuration nightmare. However you guys realized, the inheritance is the opt in, I love it.

Analogies (bad and otherwise):
- a mullet, looks short up front, on the back end goes forever.
- buying a house, everyone owns one, we've been doing it for centuries, it still takes 50 people and 60 days to make the purchase.
- setting the time on your vcr, small task, big problem
- its mole hill, if your a giant, problem is, we're ants
Requesting Gravatar... Michael Apr 26, 2008 8:45 AM
# re: Anatomy of a "Small" Software Design Change
Real-life examples of "only"?

Client would love this shirt to be also waterproof.
This books comes in French too, right?
Can we have just one more door in this car?
The house is lovely but can you move it just a few inches south?
Darling, would you mind also to keep an eye on kids while I am getting my PhD?
Requesting Gravatar... Kevin Gabbert Apr 28, 2008 10:35 AM
# re: Anatomy of a "Small" Software Design Change
I've never heard "only", I always get the dreaded:

"How hard would it be...."
Requesting Gravatar... Karl Apr 30, 2008 5:44 AM
# re: Anatomy of a "Small" Software Design Change
Thank you for sharing your thoughts Phil. You are always very interesting. I certainly agree about the issue of edge cases.

Best wishes
Requesting Gravatar... Ian Suttle May 06, 2008 12:58 AM
# re: Anatomy of a "Small" Software Design Change
The first business-like analogy I can think of is absolutely anything that sounds simple yet requires Legal to get involved. Say goodbye to a fast turnaround, and hello to "hey, what ever happened to that project?"
Requesting Gravatar... Rob May 26, 2008 10:16 AM
# re: Anatomy of a "Small" Software Design Change
I've been writing software four about thirty years now. During that time I've always been searching for the silver bullet to make my projects run smoothly and result in a high quality product. I tried many languages, frameworks and integrated application development environments. I looked at different design methodologies, diagramming techniques and paradigms. I got all involved in CMM levels, continuous process improvement, six sigma, ect. at the many companies I've worked for.

Now I develop and operate small small web-enabled applications for clients. I only take on projects I can handle myself using one very flexible tool; Lotus Notes/Domino.

I know, I know ... you're all rolling your eyes and groaning. I hated it at first, thinking they were trying to be all things to all people ... and they were. But I got past that and now I use Domino for one thing only; web application development/operation. I've never found a more flexible, simple and complete set of tools that lets me create, operate and maintain small web applications.

Even though IBM/Lotus has sadly neglected the further development of the dynamic HTML generation for many years, I've still found nothing that can replace this set of tools. It just hits a sweet spot, at least for me, that I just love.

Now for the final heresy; I make changes to the production code. That's right. I know the conventional wisdom is to make the change, test the change, them merge it into production very carefully. Well, Notes/Domino makes it easy for me to make changes to the production system without the users even noticing it, in most cases. That is until some new feature just magically appears.

This all lets me keep costs at an absolute minimum while responding to feature requests and bug fixes sometimes in just minutes. I often add a new feature for a client while they're still on the phone with me. It doesn't get any better (or more fun) than that!

Peace,

Rob:-]

p.s. And yes, I'm aware this won't work on larger projects written in compiled languages (however, most of my code is written in Java that runs in the server side within the Domino environment.) rs

What do you have to say?

(will show your gravatar)
Please add 5 and 6 and type the answer here: