2
\$\begingroup\$

I created a class to operate with concrete table and I want to generalize this code to develop more universal class.

Here is one method of this class:

public void FillColumns(DataTable table, OracleDataReader reader)
{
  if (reader != null)
    if (reader.HasRows)
    {
      table.Rows.Clear();
      while (reader.Read()) table.Rows.Add
          (
            reader["ColumnName1"],
            reader["ColumnName2"],
            reader["ColumnName3"],
            reader["ColumnName4"]
          );
    }
}

How can I refactor this concrete method, to make concrete column names as input parameters (even independing of number of columns)?

\$\endgroup\$
  • \$\begingroup\$ Yes, I'm talking about only one method which I want to refactor. \$\endgroup\$ – Gordon Freeman Apr 24 '17 at 10:36
  • \$\begingroup\$ Does it has to be a DataTable? Comparing with POCOs, DataTable is a bit slower. \$\endgroup\$ – Kosala Nuwan Perera Apr 24 '17 at 11:47
3
\$\begingroup\$

params (version 1)

You can use the same trick the Rows.Add method uses namely the params keyword

By using the params keyword, you can specify a method parameter that takes a variable number of arguments.

and pass columns names as the last parameters. Then you read the values form the reader with some LINQ and turn it into an array because it's just that, an array (but one that is created dynamically if each value is passed separately). Besides you don't need two ifs, use the and && operator instead.

public void FillColumns(
    DataTable table, 
    OracleDataReader  reader, 
    params string[] columnNames)
{
    if (reader != null && reader.HasRows)
    {
        table.Rows.Clear();
        while (reader.Read()) 
        {
            table.Rows.Add(columnNames.Select(name => reader[name]).ToArray());
        }
    }
}

Example call:

.FillColumns(table, reader, "Column1", "Column2");

table.Columns (version 2)

the second approach would be to read the column names from the data table

public void FillColumns(DataTable table, OracleDataReader reader)
{
    if (reader != null && reader.HasRows)
    {
        table.Rows.Clear();
        while (reader.Read())
        {
            table.Rows.Add(
                table
                .Columns
                .Cast<DataColumn>()
                .Select(column => reader[column.ColumnName])
                .ToArray());
        }
    }
}

table.Load (version 3)

and the third to just load the datatable from the reader:

public void FillColumns(DataTable table, OracleDataReader reader)
{
    if (reader != null && reader.HasRows)
    {
        table.Rows.Clear();
        table.Load(reader);        
    }
}
\$\endgroup\$
  • \$\begingroup\$ Thank you! I'll try to understand your examples of code ... \$\endgroup\$ – Gordon Freeman Apr 24 '17 at 11:10
  • \$\begingroup\$ The reason I complicated construction of if (reader != null && reader.HasRows) I thought that reader.HasRows throws the exception if object is null \$\endgroup\$ – Gordon Freeman Apr 24 '17 at 11:11
  • \$\begingroup\$ @GordonFreeman I guess your background is VB? Because there the_old_ And operator works that way. I think the new one is AndAlso and behaves like the C# &&. \$\endgroup\$ – t3chb0t Apr 24 '17 at 11:28
  • \$\begingroup\$ Thanks a lot! Your first variant is most suitable for me. \$\endgroup\$ – Gordon Freeman Apr 25 '17 at 5:23
  • \$\begingroup\$ (my background is Delphi) \$\endgroup\$ – Gordon Freeman Apr 25 '17 at 5:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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