Clarity over Cleverness: Follow Up, and a Practical Example

I had a nice discussion with a coworker about my last post, and wanted to add some clarity: specifically, the topic was about a piece of unit test code that looked something like this:

        public class UnitTestHere()
        {
                ...
                Db.CreateThing();
                ...
        }

        public static int CreateThing(this IDbCommand command,
                Action<Thing> modifier = null)
        {
            var thing = new Thing
                                    {
                                        ...
                                    };

            if (modifier != null)
            {
                modifier(thing);
            }

            command.Insert(thing);

            return (int) command.GetLastInsertId();
        }

So that, to modify the default values for the 'Thing' in the unit test, users had to do this:

        public class UnitTestHere()
        {
                ...
                Db.CreateThing(t =>
                        t.someField = someNonDefaultValue);
                ...
        }

My argument was along the lines of 'I don't want to have to do this to write my unit test'.

To which I got the reply (very correctly, I might add) that if I don't know it, then I should use it as a learning opportunity. Which is very true. In my case, the argument was less that I didn't know it and more that I felt the delegate was fancy-clever-overkill.

In this case, the code is both clever and clear - it serves a valid purpose, allows people reading the unit tests to see very obviously what values are being set IN the test, and it's dynamic enough to handle changes to any and all value changes in the inserted 'Thing' that might come up in future tests (like the one I was writing that brought about this discussion, for example). The hurdle of writing it is pretty quickly solved, and once the code is there it is clear enough to be maintained / changed with little effort.

I never meant to imply that more advanced tools don't have a place, just that sometimes people use them when they may not be better than the more basic options available to them.

And with that, I'm dropping this one. Happy Coding!

Coding at a Gradeschool Level

There are plenty of articles available for advice on writing clean and clear code. Many of the general principles are easy to grasp: short methods, straightforward logic and flow, clear and unambiguous variable names, consistent terminology usage, etc.

All of these things make your code easier to read, to understand, and ultimately to maintain and change. A good programmer knows that writing something clever and cool doesn't necessarily mean it was written the best way, even if it makes the code smaller. There are certainly limits to this (no one would argue much if you turns a 200 line class into 2 lines, even if those 2 lines weren't the clearest in the world, for example) but the point stands. There are 1000 ways to write code for a given feature, and there is no one true 'right' way for anything.

Which brings me to my point: a lot of times, writing something 'clever' may not be as good in the long run as writing something 'simple'. That abstract class filled with generic methods and callback delegates may get the job done and then some, may allow you to easily extend it and do anything you might ever have to do, but it's probably not easy to pick up and understand at a glance. We can set aside the benefits/risks of violating YAGNI in my example; it's worth addressing, but that's not my focus here.

In many cases, I try to keep my code as simple as I can. The analogy I try to about is a newspaper; in order to reach and be read and understood by the masses, most papers will dumb down their verbiage in order to be better understood. Similarly, I try and write my code so that a majority of it can be picked up and read by entry level programmers.

That's not to say don't used advanced programming techniques, but I know a lot of people who will use them because they are 'neat' or 'clever' or any other word that describes using it because it's interesting and not necessarily because it is the right tool for the job. There is nothing wrong with an if-else, a switch, or a foreach loop, and basic overloading or overriding of methods can get you very far. The keys to clear code lie in intelligent naming and intuitive methods.

Few people will look at this and not get the basic idea of what you are trying to do:

        public void OnClick(int id)

        {

       var ball = GetClickedBallById(id);

       ball.Roll(Units.Yards, 10, Direction.East);

        }

        private Ball GetClickedBallById(int id)

        {

       foreach(var ball in ballList)

       {

       if (ball.Id == id)

       return ball;

       }

       return null;

        }

But some, especially new programmers or those unfamiliar with the code, might scratch heads at something like this:

        

        public void Clicker(int i)

        {

       Get<int>(i).Execute(2, 10, 1);

        }

        private BaseClass Get<T>(T i)

        {

       return objList

                .Where(o => o.Id == i)

                .FirstOrDefault();

        }

Now, I'm not saying I hate generics or Linq, because actually I don't, and they definitely have a place and can be very intuitive. This is just an example, and an observation that more people will quickly understand a foreach loop with a couple of if statements than a possibly complex where clause in a Linq lambda. I know in reviewing my own code after the fact, sometimes what seemed like a really helpful and clever replacement would come back and bite me when I needed to tweak some minor functionality in that loop that I hadn't considered originally.

Every developer should ask themselves some questions in the above example: does killing those 4-5 lines make the code harder to read? Is there any way I can fix that? Is there a middle ground between clarity and concise code? How much do I expect my programmers to understand when they pick up the code? What about the BAs, or managers?

Don't be afraid to dumb down in the interest of clarify. There's nothing stopping you from refactoring it later into some super-generic-awesomeness if you REALLY feel you need it, and that it actually helps your code.

Happy coding!

Using SQL xQuery to Select Multiple Node Values

A recent task involved us having to grab information out of an rdl whose dataset was saved as xml in a sql table (Yes, you can shudder at that statement, it's ok, I know I did). This was my first foray into xQuery in a sproc, so it was a pretty interesting experience.

There was existing code for dealing with a single value (handily, as a 'value' command), but not much on selecting multiple rows. What we wound up doing was a select from the table using a cross apply to grab the nodes we cared about, then performing a value on the returned data. Like this:

SELECT temp.dataStuff.value('.','varchar(50)')
  FROM XmlTable xt
CROSS APPLY xt.xml.nodes('(/root/nodes/data)') as temp(dataStuff)

If you do a select, you will see the nodes we return are untyped sql; the value call just says 'grab the nodes we returned and shove them into varchars'.

So, with an XmlTable entry with an xml column of this:

<root>
<nodes>
<data>ABC</data>
</nodes>
<nodes>
<data>DEF</data>
</nodes>
<nodes>
<data>GHI</data>
</nodes>
</root>

You would return this out of SQL:

ABC
DEF
GHI


Another helpful source for this is here: http://stackoverflow.com/questions/1393250/getting-multiple-records-from-xml-column-with-value-in-sql-server

In the linked example the cross apply wasn't necessary because they started with an xml string. Since we had to join in and grab multiples from different rows, we needed it here.

BizTalk Scripting Functiods with Same Name Methods

So a coworker had this happen to him today, and I thought it was interesting enough to share.

Create a BizTalk map with two different C# scripting functoids, both with the same method name and input parameters, like this:

System.String GetMe(System.String input)
{
return System.String.Format("Test 1: {0}", input);
}

System.String GetMe(System.String input)
{
return System.String.Format("Test 2: {0}", input);
}

Now link them to and from some placeholder xml schemas. I won't bother detailing the boiler-plate code for that here, if you know BizTalk I'm assuming you can link two schemas using a map.

You won't get an error, but the map won't function as expected: VS/BizTalk never catches the fact that there is an invalid overloaded method in the code attached to the map. What will happen is BizTalk will execute the first valid method it finds (in this case, 'Test 1' will probably appear in both outputs).

Hopefully this will help some figure out the cause of their strange map behavior!