Defensive programming

By rickvdbosch

February 8, 2007 - 5 minutes read - 946 words

We all encountered them in the field somewhere along the way: pieces of code that are based on assumptions. There’s something fundamentally wrong in assuming when writing code. I can understand that, looking at the big picture, you sometimes have to make an assumption when developing software. Some things you just can’t know for sure. This might concern something simple like the resolution of the targeted audience. But most of the time these kinds of assumptions are based on experience or plain old common sense. When these assumptions fail to be true, most of the time a mitigation strategy is at hand. And the software usually keeps running.

When it comes to actually writing lines of code, I think you should always check and never assume. I tend to compare it to riding on a motorcycle. My instructor teached me to drive defensively. To ‘Expect the unexpected’. Sure, you can assume there won’t come a kid running from inbetween the two parked cars, but why take the risk? It’s better to anticipate a kid running across the street, so you can react accordingly when it does. And if it doesn’t? You at least didn’t take a chance …

Sometimes, an assumption is obvious:

<TD>
  <PRE>1<BR />2<BR />3<BR />4<BR /> </PRE>
</TD>

<br /> 

<TD>
  <PRE><SPAN><SPAN>public</SPAN> <SPAN>string</SPAN> ConvertObjectToString(<SPAN>object</SPAN> objectToConvert)<BR />{<BR />    <SPAN>return</SPAN> objectToConvert.ToString();<BR />}</SPAN></PRE>
</TD>

Here, you assume the object which is passed in will always contain an actual object. But when a null reference is passed in, this method will throw an exception because objectToConvert is not set to an instance of an object and therefore you can’t invoke ToString(). When this method is written ‘defensively’, it looks something like this*:

<TD>
  <PRE>1<BR />2<BR />3<BR />4<BR />5<BR />6<BR />7<BR />8<BR />9<BR />10<BR />11<BR />12<BR />13<BR />14<BR />15<BR /> </PRE>
</TD>

<br /> 

<TD>
  <PRE><SPAN><SPAN>public</SPAN> <SPAN>string</SPAN> ConvertObjectToString(<SPAN>object</SPAN> objectToConvert)<BR />{<BR />    <SPAN>string</SPAN> returnValue;<BR /><BR /><BR /><BR /><BR /><BR /><BR /><BR /><BR /><BR /><BR />    <SPAN>if</SPAN> (<SPAN>null</SPAN> == objectToConvert)<BR />    {<BR />        returnValue <SPAN>=</SPAN> <SPAN>string</SPAN>.Empty;<BR />    }<BR />    <SPAN>else</SPAN><BR />    {<BR />        returnValue <SPAN>=</SPAN> objectToConvert.ToString();<BR />    }<BR /><BR />    <SPAN>return</SPAN> returnValue;<BR />}</SPAN></PRE>
</TD>

That was an easy one. Sometimes, assumptions in code aren’t very transparent. Let’s take a look at this sample:

<TD>
  <PRE>1<BR />2<BR />3<BR />4<BR />5<BR />6<BR />7<BR />8<BR />9<BR />10<BR />11<BR />12<BR />13<BR />14<BR />15<BR />16<BR />17<BR />18<BR />19<BR />20<BR /> </PRE>
</TD>

<br /> 

<TD>
  <PRE><SPAN><SPAN>/// <summary></SPAN><BR /><SPAN>/// Method that 'does something' to a Control.</SPAN><BR /><SPAN>/// </summary></SPAN><BR /><SPAN>/// <param name=&#8221;controlToWorkOn&#8221;>Control on which 'someting will be done'</param></SPAN><BR /><SPAN>/// <returns>A Control on which 'something has been done'</returns></SPAN><BR /><SPAN>public</SPAN> Control DoSomething(Control controlToWorkOn)<BR />{<BR />    <SPAN>if</SPAN> (<SPAN>null</SPAN> !<SPAN>=</SPAN> controlToWorkOn)<BR />    {<BR />        <SPAN>if</SPAN> (controlToWorkOn <SPAN>is</SPAN> TextBox)<BR />        {<BR />            <SPAN>//Do something important to a TextBox</SPAN><BR />        }<BR />        <SPAN>else</SPAN> <SPAN>if</SPAN> (controlToWorkOn <SPAN>is</SPAN> Button)<BR />        {<BR />            <SPAN>//Do something important to a Button</SPAN><BR />        }<BR />    }<BR />    <SPAN>return</SPAN> controlToWorkOn;<BR />}</SPAN></PRE>
</TD>

We will leave the ‘do something important to…’ parts out of this. The Control is checked for being null. The Control is typechecked so no wrong casts will be made. So nothing seems wrong, right? The problem here is the fact that you know this method only does something for a TextBox or a Button. But what happens if a year from now somebody gets the assignment to change the application, and he or she doesn’t know about the method only working for TextBoxes or Buttons? The method will return the Control unchanged, but the caller might think everything is OK. Often we write methods with certain constraints about the calls made to that method in the backs of our heads. And often without realy realizing it. But there might come a time that somebody who doesn’t know the constraints has to jump in.
To be sure passing in another type of Control will not give any problems, you might consider a solution like this one:

<TD>
  <PRE>1<BR />2<BR />3<BR />4<BR />5<BR />6<BR />7<BR />8<BR />9<BR />10<BR />11<BR />12<BR />13<BR />14<BR />15<BR />16<BR />17<BR />18<BR />19<BR />20<BR />21<BR />22<BR />23<BR />24<BR /> </PRE>
</TD>

<br /> 

<TD>
  <PRE><SPAN><SPAN>/// <summary></SPAN><BR /><SPAN>/// Method that 'does something' to a Control.</SPAN><BR /><SPAN>/// </summary></SPAN><BR /><SPAN>/// <param name=&#8221;controlToWorkOn&#8221;>Control on which 'someting will be done'</param></SPAN><BR /><SPAN>/// <returns>A Control on which 'something has been done'</returns></SPAN><BR /><SPAN>public</SPAN> Control DoSomething(Control controlToWorkOn)<BR />{<BR />    <SPAN>if</SPAN> (<SPAN>null</SPAN> !<SPAN>=</SPAN> controlToWorkOn)<BR />    {<BR />        <SPAN>if</SPAN> (controlToWorkOn <SPAN>is</SPAN> TextBox)<BR />        {<BR />            <SPAN>//Do something important to a TextBox</SPAN><BR />        }<BR />        <SPAN>else</SPAN> <SPAN>if</SPAN> (controlToWorkOn <SPAN>is</SPAN> Button)<BR />        {<BR />            <SPAN>//Do something important to a Button</SPAN><BR />        }<BR />        <SPAN>else</SPAN><BR />        {<BR />            <SPAN>throw</SPAN> <SPAN>new</SPAN> NotSupportedException(<SPAN>&#8220;Control-type not supported in this method&#8221;</SPAN>);<BR />        }<BR />    }<BR />    <SPAN>return</SPAN> controlToWorkOn;<BR />}</SPAN></PRE>
</TD>

Another way to solve this problem is to make the DoSomething method private and generate two public methods. One that takes a TextBox and one that takes a Button, both eventually calling the private DoSomething method. This would actually be the solution I would choose, because then the problem pops up compile time in stead of runtime. And I think it aids future developers better.

‘Defensive programming’. Writing software anticipating as many problems as possible to ensure correct functioning of the software. Even when the software is edited later by someone who doesn’t have your mindset.

Edit: Wikipedia has an article on defensive programming.
Edit2: Thanks Geraldo Hockx for your sharp eye! I changed the code samples: I made a few typo’s.

* Yes, you could initialize the local parameter to string.Empty and only have an if-statement. And yes, you could use the conditional operator. And of course you could use two return statements. But this is my coding style (at the moment) so this is how I would do it right now ;).

dotnet
HowTo