Friday, April 9, 2010

Wrapping code: Good practice?

I wrap native code because:
  • Wrapping makes it extensible.
  • It's easier to remember.
  • The native code is absurdly lengthy / wordy / etc.
  • Code in the library will never be lost.
In the halls of computer science (a discipline that hadn't been invented when I was in college), do deep-thinking people ever advocate the idea that calling native code directly is a *bad practice*? That code should be wrapped in a method, to make it extensible?

Here's an example I ran into last night:

I've written a utility for executing miscellaneous C# methods, which has a f.OpenFile() helper method that wraps Process.Start(). Pass in a file name and it opens using the associated application. f.OpenFile() adds a *little* functionality, throwing an exception that logs the file name, when the file doesn't exist. So far, the method is called about a hundred places.

Last night I enhanced it to prompt the user when the file doesn't exist at the passed path, allowing the user to browse to the file's location, and applying that location going forward. If the alt-key is down when the utility method is run, the user gets the prompt regardless of whether the path is valid -- so the user can point the method at a different path. That let's me change the path without going into the code, a time-saver.

I could not have done that (without difficulty) if I'd had 100+ Process.Start() calls in my code. With f.OpenFile(), I only had to change the code in one place. It was easy to add the new feature because ProcessStart() was *wrapped*. I had a hook. It made Process.Start() extensible, in some sense.

Over the years, this pattern has occurred often enough, and *so usefully* -- that I wonder if it has been recognized with a special name??

Another reason I wrap code is because my brain has tired RNA. I can't possibly remember this:

using (StreamReader streamReader =
new StreamReader(HttpWebRequest.Create(_theUrl).GetResponse().GetResponseStream()))
return streamReader.ReadToEnd();

But I can remember the wrapper method f.GetUrlsHtml(). A few months ago, I refactored f.GetUrlsHtml(), and I *could* refactor it, because it was wrapped, instead of being native code in a couple dozen locations. I suspect this may be considered to be a bad practice, because *so often* I see the same tired block of code repeated and repeated, copied and pasted. Doesn't DRY imply *"Wrap it!"*?

Wrapping seems particularly appropriate, when it takes too many lines to do something simple.

f.ExecuteCommand(@"%SystemRoot%\system32\restore\rstrui.exe", 0);

The method executes about 10 lines -- which *begs* for wrapping, to me.

And yet another reason to wrap: laziness and/or esthetics.

if (String.IsNullOrEmpty( aStringVar1 ) || String.IsNullOrEmpty( aStringVar2 )) ...

To me, String.IsNullOrEmpty() seems ridiculously long. It's tedious and distracting. I wrap it:

if (f.ns(aString1) || f.ns(aString2))

The "ns" stands for Non-String. Once, it just called String.IsNullOrEmpty(), but now f.ns() returns true if the string is all *whitespace*. When you're validating a string, don't you want to catch it, when the calling code passes a single space? With f.ns(), I can set a breakpoint to catch where in the code an invalid string is passed.

Could that be done with String.IsNullOrEmpty()? I'd like to know; that could be very helpful.

Another reason for wrapping code is so it doesn't get lost. A friend emailed slick code applying LINQ to subtract one List from another (T being a simple type). I wrapped it in a method, so I could test it, and so it would be there when I needed it... which wasn't too long in coming. I routinely wrap code that I find on the web, that I'll find useful someday. It will never be lost if it's in my library.

I guess I could put code in a repository for that purpose (anyone have a recommendation?), but then I'd have to *find* it. It's easy to find in Visual Studio.

An argument *against* wrapping is that the code is unfamiliar, because I invented it, so no one else will understand it. f.ns() is obscure, I admit. "Right-click, Go To Implementation" is pretty easy, though. The name f.GetUrlsHtml() is more transparent than the code it wraps, right? Maybe f.GetHtmlFromUrl() would be better (I love VS's rename!!).

I guess using the methods creates a dependency on the method library. OK. I'll deal.

Are there other reasons why wrapping frequently-used code is a bad practice?

I'm really intent on getting better at this. Your responses are appreciated.

- Hoytster

No comments:

Post a Comment