A better way to modify tables

Let's go back to our example here and fix one of my pet peeves from earlier: the usability of the helper function is less than spectacular: there is too much knowledge of Windows Installer table information, making this unwieldy. Let's see if we can fix this up in some capacity.

Let's start with ModifyTableData. This method is currently called with a Database parameter, an extensive SQL statement, as well as a column index and a new value. I like the bookend parameters (for now), but let's see how we could improve the SQL statement and column index parameters.

First, column index (since that is the easiest): instead of a column index, it would be better to make this a column name. That will make code calling the function much more readable.

Next, let's look at the anatomy of the SQL statement. I'll use one of the statements from EnableLaunchApplication as a "typical" example:

"SELECT `Dialog_`, `Control`, `Type`, `X`, `Y`, `Width`, `Height`, `Attributes`, `Property`, `Text`, `Control_Next`, `Help` FROM `Control` WHERE `Dialog_`='FinishedForm' AND `Control`='BannerBmp'"

We could also summarize the statement like this:

"SELECT {A bunch of column names} FROM {Table name} WHERE {Specific column entries}"

So, let's plan to modify our method signature to take these 3 elements into account, instead of the full SQL statement.

Let's see what this would look like when we view the modified ModifyStringTableDataTest code. Here is the old code:

 sql = string.Format("SELECT `Error`, `Message` FROM `Error` WHERE `Message`='{0}'", message);
string newMessage = "A new message";
Utilities.ModifyTableData(msi, sql, 2, newMessage);

And here is the new code:

 string newMessage = "A new message";
Utilities.ModifyTableData(msi, "Error", "Message", newMessage, string.Format("`Message`='{0}'", message));

This of course results in a compiler error:

Error No overload for method 'ModifyTableData' takes '5' arguments

So let's fix the compiler errors (and try to fix up the function in the same step). This may be a relatively simple change; I guess we'll find out in a minute:

 public static void ModifyTableData(Database msi, string tableName, string columnName, object value, string whereClause)
{
    string sql = string.Format("SELECT `{0}` FROM `{1}` WHERE {2}", columnName, tableName, whereClause);
    View view = msi.OpenView(sql);
    view.Execute(null);

    Record record = view.Fetch();
    if (value is int)
    {
        record.set_IntegerData(1, (int)value);
    }
    else
    {
        record.set_StringData(1, value.ToString());
    }

    view.Modify(MsiViewModify.msiViewModifyReplace, record);
    view.Close();

    System.Runtime.InteropServices.Marshal.FinalReleaseComObject(record);
    System.Runtime.InteropServices.Marshal.FinalReleaseComObject(view);
}

The first step here is to duplicate the old SQL statement from before, except instead of selecting all of the columns in the table, I select only the relevant column. This affects my call to the set data calls, allowing me to use index 1 instead of a parameter value. Note I also re-ordered the parameters a bit. The first three seem natural: keep drilling down in scope (msi, table, column). I wanted to have the new value immediately follow the column name, which left the where clause for the end.

I'd like to run the tests to make sure the code works, but of course I have to modify all the calls to ModifyTableData (note: I realized after starting work on this blog that I was still using the old ModifyTableData method from the EnableLaunchApplication. I have corrected that in this issue). After finishing off those, run the tests. Good news, everyone: all of the tests still pass! And running the EnableLaunchApplication task also works and resultant msi shows that things are still working there as well.

So, was this really an improvement? I think so: the primary improvement is code readability. For example, which is easier to read? This:

 Utilities.ModifyTableData(Msi, "SELECT `Dialog_`, `Control`, `Type`, `X`, `Y`, `Width`, `Height`, `Attributes`, `Property`, `Text`, `Control_Next`, `Help` FROM `Control` WHERE `Dialog_`='FinishedForm' AND `Control`='BannerBmp'", 11, "CheckboxLaunch");

Or this:

 Utilities.ModifyTableData(Msi, "Control", "Control_Next", "CheckboxLaunch", "`Dialog_`='FinishedForm' AND `Control`='BannerBmp'");

Since the first probably doesn't even fit on the screen, I'm going to have to go with the latter. Of course, we now know I could have simplified the method call to read:

 Utilities.ModifyTableData(Msi, "SELECT `Control_Next` FROM `Control` WHERE `Dialog_`='FinishedForm' AND `Control`='BannerBmp'", "CheckBoxLaunch");

But I still like my change better: it seems less error prone because we avoid much of the SQL formatting that has to be done.

And of course, I have other reasons as well (stay tuned).

MWadeBlog_07_04_04.zip