3
\$\begingroup\$

I'm trying to get a better understanding of decoupling methods. Right now, I have this method:

private bool ContainsLegalFirstName(DataRow row, string legalFirstNameColumn)
{
    return row.Table.Columns.Contains(legalFirstNameColumn)
        && !String.IsNullOrEmpty(row[legalFirstNameColumn].ToString());
}

I was thinking about changing the DataRow to an IDataRecord and adding an IListSource parameter and passing in the DataTable, but then the problem is I can't access the columns of the DataTable from the IListSource. Any suggestions or is what I have good enough?

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

As DataRow does not implement IDataRecord, you would have to pass an adapter that wraps the row and implements the interface. Also, passing an IListSource would only help if it always returned a ITypedList (which is not guaranteed) which you could then query for available properties by name. But what would be the advantage of all that?

In my opinion, the parameter types are alright. We are still talking about a private helper method, not one that needs to be accessible using all kinds of interfaces. There are, however, some things that I might change:

    • Either: Rename the method to represent that it actually does not care about the legal first name. You could currently rename it to ContainsLegalLastName without any change in behaviour.
    • Or: Remove the legalFirstNameColumn and get that value from somewhere else (some configuration, const, …).
  1. Unless you decide to remove the column name parameter and get the column name from an instance variable, make the method static. It currently does not use any state at all.
\$\endgroup\$
4
  • \$\begingroup\$ Rename the method to represent that it actually does not care about the legal first name - Why do you say that? Remove the legalFirstNameColumn and get that value from somewhere else (some configuration, const, …). - When I call this method, I pass a const to it. That const varies depending on the DataTable that is being validated by this method (there are 5 of them with different column names). Is that okay? \$\endgroup\$
    – bmay2
    Commented Jul 6, 2014 at 15:18
  • \$\begingroup\$ Yes, but I think you could still rename the method, because the fact that you are checking for the last name does not really matter, since the correct column name is determined outside the method in question. \$\endgroup\$
    – hangy
    Commented Jul 6, 2014 at 15:38
  • \$\begingroup\$ Ah, I see what you mean. So maybe rename to ContainsColumn() or something and pass the column name? \$\endgroup\$
    – bmay2
    Commented Jul 6, 2014 at 16:31
  • \$\begingroup\$ Yes, sorry if I was not clear enough in my answer. :) \$\endgroup\$
    – hangy
    Commented Jul 7, 2014 at 8:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.