StackOverflow2013

Note that there are some explanatory texts on larger screens.

plurals

POHow defensively should I program?
text
Body
copied!<p>i was working with a small routine that is used to create a database connection:</p> <h2>Before</h2> <pre><code>public DbConnection GetConnection(String connectionName) { ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName]; DbProviderFactory factory = DbProviderFactories.GetFactory(cs.ProviderName); DbConnection conn = factory.CreateConnection(); conn.ConnectionString = cs.ConnectionString; conn.Open(); return conn; } </code></pre> <p>Then i began looking into the .NET framework documentation, to see what the <strong>documented</strong> behaviour of various things are, and seeing if i can handle them.</p> <p>For example:</p> <pre><code>ConfigurationManager.ConnectionStrings... </code></pre> <p>The <a href="http://msdn.microsoft.com/en-us/library/system.configuration.configurationmanager.connectionstrings.aspx" rel="noreferrer">documentation</a> says that calling <strong>ConnectionStrings</strong> throws a <a href="http://msdn.microsoft.com/en-us/library/system.configuration.configurationerrorsexception.aspx" rel="noreferrer">ConfigurationErrorException</a> if it could not retrieve the collection. In this case there is nothing i can do to handle this exception, so i will let it go.</p> <hr> <p>Next part is the actual indexing of the <strong>ConnectionStrings</strong> to find <em>connectionName</em>:</p> <pre><code>...ConnectionStrings[connectionName]; </code></pre> <p>In this instance the <a href="http://msdn.microsoft.com/en-us/library/c6t3b6f3.aspx" rel="noreferrer">ConnectionStrings documentation</a> says that the property will return <strong>null</strong> if the connection name could not be found. i can check for this happening, and throw an exception to let someone high up that they gave an invalid connectionName:</p> <pre><code>ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName]; if (cs == null) throw new ArgumentException("Could not find connection string \""+connectionName+"\""); </code></pre> <hr> <p>i repeat the same excercise with:</p> <pre><code>DbProviderFactory factory = DbProviderFactories.GetFactory(cs.ProviderName); </code></pre> <p>The <a href="http://msdn.microsoft.com/en-us/library/h508h681.aspx" rel="noreferrer">GetFactory</a> method has no documentation on what happens if a factory for the specified <code>ProviderName</code> couldn't be found. It's not documented to return <code>null</code>, but i can still be defensive, and <strong><em>check</em></strong> for null:</p> <pre><code>DbProviderFactory factory = DbProviderFactories.GetFactory(cs.ProviderName); if (factory == null) throw new Exception("Could not obtain factory for provider \""+cs.ProviderName+"\""); </code></pre> <hr> <p>Next is construction of the DbConnection object:</p> <pre><code>DbConnection conn = factory.CreateConnection() </code></pre> <p>Again the <a href="http://msdn.microsoft.com/en-us/library/system.data.common.dbproviderfactory.createconnection.aspx" rel="noreferrer">documentation</a> doesn't say what happens if it couldn't create a connection, but again i can check for a null return object:</p> <pre><code>DbConnection conn = factory.CreateConnection() if (conn == null) throw new Exception.Create("Connection factory did not return a connection object"); </code></pre> <hr> <p>Next is setting a property of the Connection object:</p> <pre><code>conn.ConnectionString = cs.ConnectionString; </code></pre> <p>The docs don't say what happens if it could not set the connection string. Does it throw an exception? Does it ignore it? As with most exception, if there was an error while trying to set the ConnectionString of a connection, there's nothing i can do to recover from it. So i'll do nothing.</p> <hr> <p>And finally, opening the database connection:</p> <pre><code>conn.Open(); </code></pre> <p>The <a href="http://msdn.microsoft.com/en-us/library/system.data.common.dbconnection.open.aspx" rel="noreferrer">Open method</a> of DbConnection is abstract, so it's up to whatever provider is descending from DbConnection to decide what exceptions they throw. There is also no guidance in the abstract Open methods documentation about what i can expect to happen if there's an error. If there was an error connecting, i know i cannot handle it - i'll have to let it bubble up where the caller can show some UI to the user, and let them try again.</p> <hr> <h2>After</h2> <pre><code>public DbConnection GetConnection(String connectionName) { //Get the connection string info from web.config ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName]; //documented to return null if it couldn't be found if (cs == null) throw new ArgumentException("Could not find connection string \""+connectionName+"\""); //Get the factory for the given provider (e.g. "System.Data.SqlClient") DbProviderFactory factory = DbProviderFactories.GetFactory(cs.ProviderName); //Undefined behaviour if GetFactory couldn't find a provider. //Defensive test for null factory anyway if (factory == null) throw new Exception("Could not obtain factory for provider \""+cs.ProviderName+"\""); //Have the factory give us the right connection object DbConnection conn = factory.CreateConnection(); //Undefined behaviour if CreateConnection failed //Defensive test for null connection anyway if (conn == null) throw new Exception("Could not obtain connection from factory"); //Knowing the connection string, open the connection conn.ConnectionString = cs.ConnectionString; conn.Open() return conn; } </code></pre> <h2>Summary</h2> <p>So my four line function, became 12 lines, and required 5 mintues of documentation lookup. In the end i did catch one case where a method is allowed to return null. But in practice all i did was convert an access violation exception (if i attempts to call methods on a null reference) into an <strong>InvalidArgumentException</strong>. </p> <p>i also catch two possible cases where there could be <strong>null</strong> return objects; but again i only traded one exception for another. </p> <p>On the positive side, it did catch two problems, and explain what happened in the exception message, rather than bad things happening down the road (i.e. the buck stops here)</p> <p>But is it worth it? Is this overkill? Is this defensive programming gone awry? </p>

Querying!

Guidance

An individual column

Larger individual text columns get their own page to allow for proper reading.

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload