Is there any performance benefit with "chaining" statements in .NET?

Question

When retrieving a lookup code value from a table, some folks do this...

Dim dtLookupCode As New LookupCodeDataTable()
Dim taLookupCode AS New LookupCodeTableAdapter()
Dim strDescription As String

dtLookupCode = taLookupCode.GetDataByCodeAndValue("EmpStatus", "FULL")
strDescription = dtLookupCode.Item(0).Meaning

...however, I've also seen things done "chained" like this...

strDescription = taLookupCode.GetDataByCodeAndValue("EmpStatus", "FULL").Item(0).Meaning

...which bypasses having a lookup code data table in the first place since the table adapter knows what the structure of its result set looks like.

Does using the "chained" method save the overhead of creating the data table object, or does it effectively get created anyway in order to properly handle the .Item(0).Meaning statement?

Answer 1

Yeah, don't say "inline" because that means something specific in other languages. Most likely the performance difference is either zero or so small it doesn't matter, it's just a matter of preference. Do you want to write it out in separate statements to make it more clear, or write it all on one line to type it out quicker?

Answer 2

Those two lines would compile down to the same thing. I would pick whichever one is easier for you read. Inlining normally refers to something a bit different.

Answer 3

The structure is still created, you just do not have a reference for it.

Answer 4

Usually it just makes the code less readable.

And often, when people use this "inlining" (i.e. chaining), they'll re-access a property or field of a class multiple times instead of getting it just once and storing in a local variable. This is generally a bad idea because one doesn't usually know how that field or property is returned. For example, it may be calculated each time, or it may be calculated once and stored privately in the class.

Here are two illustrations. The first snippet is to be avoided:

if (ConfigurationManager.AppSettings("ConnectionString") == null)
{
    throw new MissingConfigSettingException("ConnectionString");
}

string connectionString = ConfigurationManager.AppSettings("ConnectionString");

The second is preferable:

string connectionString = ConfigurationManager.AppSettings("ConnectionString")

if (connectionString == null)
{
    throw new MissingConfigSettingException("ConnectionString");
}

The problem here is that AppSettings() actually has to unbox the AppSettings collection everytime a value is retrieved:

// Disassembled AppSettings member of ConfigurationManager 

public static NameValueCollection AppSettings
{
    get
    {
        object section = GetSection("appSettings");

        if ((section == null) || !(section is NameValueCollection))
        {
            throw new
                ConfigurationErrorsException(SR.GetString("Config_appsettings_declaration_invalid"));
        }

        return (NameValueCollection) section;
    }
}

Answer 5

Debugging the latter is going to be harder if you want to see the intermediate state, and single step through the stages.

I would go for readability over the amount of screen real estate used here since performance is a wash.

Answer 6

Straying from the "inline" part of this, actually, the two sets of code won't compile out to the same thing. The issue comes in with:

Dim dtLookupCode As New LookupCodeDataTable()
Dim taLookupCode AS New LookupCodeTableAdapter()

In VB, this will create new objects with the appropriately-named references. Followed by:

dtLookupCode = taLookupCode.GetDataByCodeAndValue("EmpStatus", "FULL")

We immediately replace the original dtLookupCode reference with a new object, which creates garbage to be collected (an unreachable object in RAM).

In the exact, original scenario, therefore, what's referred to as the "inline" technique is, technically, more performant. (However, you're unlikely to physically see that difference in this small an example.)

The place where the code would essentially be the same is if the original sample read as follows:

Dim taLookupCode AS New LookupCodeTableAdapter
Dim dtLookupCode As LookupCodeDataTable
Dim strDescription As String

dtLookupCode = taLookupCode.GetDataByCodeAndValue("EmpStatus", "FULL")
strDescription = dtLookupCode.Item(0).Meaning

In this world, we only have the existing references, and are not creating junk objects. I reordered the statements slightly for readability, but the gist is the same. Also, you could easily single-line-initialize the references with something like this, and have the same basic idea:

Dim taLookupCode AS New LookupCodeTableAdapter
Dim dtLookupCode As LookupCodeDataTable = taLookupCode.GetDataByCodeAndValue("EmpStatus", "FULL")
Dim strDescription As String = dtLookupCode.Item(0).Meaning

Answer 7

This:

dtLookupCode = taLookupCode.GetDataByCodeAndValue("EmpStatus", "FULL")
strDescription = dtLookupCode.Item(0).Meaning

and this:

strDescription = taLookupCode.GetDataByCodeAndValue("EmpStatus", "FULL").Item(0).Meaning

are completely equivalent.

In the first example, you've got an explicit temporary reference (dtLookupTable). In the second example, the temporary reference is implicit. Behind the scenes, the compiler will almost certainly create the same code for both of these. Even if it didn't emit the same code, the extra temporary reference is extremely cheap.

However, I'm not sure if this line:

Dim dtLookupCode As New LookupCodeDataTable()

is efficient. It looks to me like this creates a new LookupCodeDataTable which is then discarded when you overwrite the variable in the later statement. I don't program in VB, but I would expect that this line should be:

Dim dtLookupCode As LookupCodeDataTable

The reference is cheap (probably free), but constructing an extra lookup table may not be.

Answer 8

I call it chaining.

You are asking the wrong question.

What you need to ask is: Which is more readable?

If chaining makes the code easier to read and understand than go ahead and do it.

If however, it obfuscates, then don't.

Any performance optimizations are non-existant. Don't optimize code, optimize algorithms.

So, if you are going to be calling Item(1) and Item(2), then by chaining, You'll be creating the same object over and over again which is a bad algorithm.

In that case, then the first option is better, as you don't need to recreate the adapter each time.

Answer 9

One reason against 'chaining' is the Law of Demeter which would suggest that your code is fragile in the face of changes to LookupCodeDataTable.

You should add a function like this:

function getMeaning( lookupCode as LookupCodeDataTable)
 getMeaning=lookupCode.Item(0).Meaning
end function

and call it like this:

strDescription=getMeaning(taLookupCode.GetDataByCodeAndValue("EmpStatus", "FULL"))

Now getMeaning() is available to be called in many other places and if LookupCodeDataTable changes, then you only have to change getMeaning() to fix it.

Answer 10

It's the same unless you need to refer to the returned objects by taLookupCode.GetDataByCodeAndValue("EmpStatus", "FULL") or Item(0) multiple times. Otherwise, you don't know if the runtime for this function is log(n) or n, so for the best bet, I would assign a reference to it.

Answer 11

Besides maintainability, here's another reason to avoid chaining: Error checking.

Yes, you can wrap the whole thing in try/catch, and catch every exception that any part of the chain can throw.

But if you want to validate results in between calls without try/catch, you have to split things apart. For instance:

• What happens when GetDataByCodeAndValue returns null?
• What if it returns an empty list?

You can't check for these values without try/catch if you're chaining.