# Checking whether a table has modified rows

The company I am with has a lot of old code migrated over from ALPHA Basic, written back in the 1980s. As development languages have evolved (should I say, "died off"?) the code has been migrated to newer languages.

The company also hires a lot of new graduates from a local college. Sometimes, the code is written in a textbook fashion. I see a lot of code that could be written differently.

Here is one piece of code that I have encountered in a project I maintain:

Public ReadOnly Property HasModifiedRows() As Boolean
Get
Dim dt As DataTable

dt = Nothing
Try
dt = Me.GetTableObj.GetChanges(DataRowState.Modified)
If dt IsNot Nothing Then
Return True
Else
Return False
End If
Finally
If dt IsNot Nothing Then
dt.Dispose()
dt = Nothing
End If
End Try
End Get
End Property


The code looks deliberately spelled out, step for step. This could be to help our new developers understand what is going on, or it could be code that was migrated over from older languages that required all variables be defined at the top.

I do not know why, but seeing all of the unnecessary lines of code is like fingernails on a chalkboard.

### My planned rewrite

I am constantly compelled to rewrite pieces of code like that above to this:

Public ReadOnly Property HasModifiedRows() As Boolean
Get
Using table As DataTable = GetTableObj.GetChanges(DataRowState.Modified)
Return True
End Using
Return False
End Get
End Property


### Question and concerns

• Is one design pattern better than another?
• Is there a performance difference in the two?
• Would my version with the using syntax have any unanticipated results?

As I understand the VB Using Statement, if an object enters a using block, it cannot be null and it will always dispose of itself whenever it goes out of scope. I could of course be wrong though. I do not know how to guarantee this in .Net.

Should I leave the old code alone?

If there is any benefit to rewriting it, I am certainly happier with the streamlined look.

I'd really like to get some opinions on if I am doing any good by changing the code. If I am doing harm, I certainly would like to know!

Is one design pattern better than another?

Every code statement is a potential defect. If you can express yourself in a shorter form without sacrificing readability, then by all means you should. The less code, the less to read and debug, the fewer suspicious elements.

Is there a performance difference in the two?

I don't actually know VB, but this is a common pattern in many languages, and it's clear that the performance should be the same, or the second might be even faster, thanks to some optimization magic by the compiler.

Would my version with the using syntax have any unanticipated results?

Increased understanding among readers? You changed not just the writing style, but you also gave the variables more meaningful names. It's intuitive and a lot easier to read than the first one. Both seem to accomplish exactly the same thing.

Should I leave the old code alone?

Probably. Don't fix what ain't broken. Although the original method is not implemented pretty, from the outside it doesn't really matter: it has a good name, and it does exactly what the name says and nothing else, it works. It's nice to make it nicer, but there isn't really a need. Now that you looked at it closely and know that it works, try to treat it as a black box, and remember not to open again. Use your energy for more creative tasks.

• It's a black box and leave it alone. I'll need to remember that. :) – jp2code Jun 3 '15 at 13:49