Is This Too Clever?

I was cleaning up some code in my blog software and ran into this little code pattern:

int x = f.FindSomething("a");
if (x != -1)
{
    z = x;
}

It was repeated in various places, with slight changes, in most of the class I was working on. I decided that I'd like to remove the sentinel and see if I could make things cleaner.

One alternative would be to use an int? as the return value and then use the ?? operator, like this:

x = f.FindSomething("a") ?? x;

I guess that would work, but nullable value types are a little creepy, and now the FindSomething method would have to return a null, which conflicts with the way most of the existing .NET framework behaves (i.e. returning a -1 sentinel for similar methods).

For my second attempt I created a new method and changed the name in such a way to make it clear what I was doing. And since, in most cases, I wasn't using the return value if the parameter wasn't found, I decided to use a delegate to capture the value:

f.FindSomethingThenAssign("a", n => z = n);

Now, I really like this improvement, it no longer requires the sentinel value; but, I'm worried that someone looking at this code would not immediately understand what was going on.

In the end, I've decided to stick with the Action<> argument. Here's the new method:

public void FindSomethingThenAssign(string arg, Action<int> assign)
{
    int x = search somewhere for arg;
    if (x != -1)
    {
        assign(x);
    }
}

About this entry