CodeSOD: Save Yourselves!

Scott K was cleaning up a configuration utility used by his team when he dredged up this sanity-threatening artifact:

void Save(string path){ XmlTextWriter write = null; try { write = new XmlTextWriter(path, null); } catch (IOException) { write.WriteEndDocument(); write.Close(); try { write = new XmlTextWriter(path, null); } catch (IOException) { return; } } // Write stuff to the file} 

There's nothing terribly complex or unclear about this C# function that takes a file path and attempts to open the XML file located there for writing. Like any good file-I/O-related code, Save is prepared for I/O-related problems: if, say, the filename is invalid, the catch block is ready to leap into action. The code it runs" is guaranteed to fail in three exciting ways:

1. Since the exception must have been thrown by XMLTextWriter's constructor, write is still null. Trying to call a method on it will throw a NullReferenceException
2. Even if write weren't null, the first call attempts to write a document end tag to the XML document. Since this code is executing before it ever started writing the XML document, this would throw an ArgumentException
3. Ignoring both of those, the subsequent attempt to close the unopened document using the uninstantiated object reference will throw another NullReferenceException

And just in case none of the above happens the code then re-tries the same call that threw the original exception, oblivious to the well-loved clichi(C). When this doesn't work, the method swallows the exception and returns, leaving the caller blissfully unaware that the file was not, in fact, saved.

Thank goodness none of that matters, because of Exciting Failure Case #1! And since the calling code doesn't handle exceptions, the user will be very aware their file wasn't saved when the whole application crashes! Now that's a great "save".

Bonus Content!

For additional entertainment, here's the code that calls the Save method. How long 'til the madness sets in when you contemplate what this is doing?

string temp = sfd.ToString();string[] tempArr = temp.Split(' ');Save(tempArr[4]); 

(Note: sfd is an instance of SaveFileDialog)

[Advertisement] Use NuGet or npm? Check out ProGet, the easy-to-use package repository that lets you host and manage your own personal or enterprise-wide NuGet feeds and npm repositories. It's got an impressively-featured free edition, too!