C# Repeating Functions with single line changes

Question

I have noticed that I am repeating the same few lines of code over and over and I want to refactor to prevent that for upkeep. However, the changes between each iteration is in the middle of all the validation, so I'm not sure the best way to accomplish this. I feel like there is some way to write a generic function that can accept an input to change the inner code, but the only way I can think of is an enumeration and a switch statement for each one.

Are there any better methods for making this block of repeated code more readable and maintainable?

Specific block repeated throughout:

    if (_read.HasRows)
    {
        while (_read.Read())
        {
            //DO SOMETHING...;
        }
    }
    _read.Close();

Example usages:

public List<object> TableList()
{
    List<object> list = new List<object>();

    _cmd.CommandText = "SELECT * FROM sys.tables";

    try
    {
        _read = _cmd.ExecuteReader();
    }
    catch (Exception e)
    {
                     //please ignore this try catch for now
        list[0] = e; //this is a temporary measure until I finalize error handling
    }

//block to generalize
    if (_read.HasRows)
    {
        while (_read.Read())
        {
            list.Add(_read.GetValue(0)); //unique line
        }
    }


    _read.Close();

    return list;
}

another example:

private List<object[]> _Columns = new List<object[]>()    
private bool SetColumns()
{
    _oldCommand = _cmd.CommandText;
    _cmd.CommandText = "exec sp_columns " + tableName;
        try
        {
            _read = _cmd.ExecuteReader();
        }
        catch (Exception e)
        { }  //again, ignore the messy error handling.  That is coming next.


//block to generalize
    if (_read.HasRows)
    {
        while (_read.Read())
        {
            _Columns.Add(new object[2] { _read.GetValue(3), _read.GetValue(5) }); //unique line
        }
    }



    _read.Close();
    _cmd.CommandText = _oldCommand;
    if (_Columns.Count != 0)
    {
        return true;
    }
    else
    {
        return false;
    }
}

Just a reminder at the bottom of the specific scope of the question. How can I generalize the repeated block of code while still allowing the unique lines to exist per different use? I only gave two examples, but there are several instances of this issue throughout and I would like to refactor before going to deep.

Final Implementation after seeing answers:

    /// <summary>
    /// Sets a list of tables in the connected database.
    /// </summary>
    /// <returns>Returns a List<string> of the tables.  If there is an error, then the first element of the list will conatin the exception.</returns>
    public List<string> TableList()
    {
        List<string> list = new List<string>();

        _cmd.CommandText = "SELECT * FROM sys.tables";

        ExecuteReader(_read => { list.Add(_read.GetString(0)); });

        _read.Close();

        return list;
    }


    /// <summary>
    /// sets column names and types to the _columns private variable [0] = name, [1] = type
    /// </summary>
    private void SetColumns()
    {
        _oldCommand = _cmd.CommandText;

        _cmd.CommandText = "exec sp_columns " + TableName;

        ExecuteReader(_read => columns.Add(new string[2] { _read.GetString(3), _read.GetString(5) }));

        _cmd.CommandText = _oldCommand;

        _read.Close();

    }


    /// <summary>
    /// Executes the reader with error handling and type returns.
    /// </summary>
    /// <param name="readline">the code that actually pulls the data from each line of the reader</param>
    private void ExecuteReader(Action<SqlDataReader> readline)
    {
        if (_con.State.HasFlag(ConnectionState.Closed)) { _con.Open(); }

        _read = _cmd.ExecuteReader();

        while (_read.Read())
        {
            readline(_read);
        }

        _con.Close();

    }

I chose this method as it felt the most readable with my level of experience. I have done some looking into Dapper as many others have recommended. I'll most likely be using it in future projects as I like to write my own boilerplate code at least once for my own experience sake.

Thank you everyone for the amazing answers. I hope others find this helpful in the future.

Answer 1

You can factor out the population code by passing in a lambda expression, for example:

SetColumns(reader => _Columns.Add(new object[2] { _read.GetValue(3), _read.GetValue(5) }));

SetColumns(reader => list.Add(reader.GetValue(0)));

See the updated line inside the while loop:

private List<object[]> _Columns = new List<object[]>();
private bool SetColumns(Action<DbDataReader> populateColumns)
{
    _oldCommand = _cmd.CommandText;
    _cmd.CommandText = "exec sp_columns " + tableName;
        try
        {
            _read = _cmd.ExecuteReader();
        }
        catch (Exception e)
        { }  //again, ignore the messy error handling.  That is coming next.


//block to generalize
    if (_read.HasRows)
    {
        while (_read.Read())
        {
            populateColumns(_read);
        }
    }



    _read.Close();
    _cmd.CommandText = _oldCommand;
    if (_Columns.Count != 0)
    {
        return true;
    }
    else
    {
        return false;
    }
}

Answer 2

There comes 2 ways out into the mind when you are thinking about the reuse.

• Just make it a method, to use from everywhere.
• Inheritance

Now in this case inheritance sounds a little pushy. But making a method can go right.

Here is my shot:

public class FillerFromDb
{
    public IList ListToBeFilled { get; set; }
    SqlCommand Command { get; set; }
    SqlDataReader Reader { get; set; }
    public FillerFromDb ( IList listToBeFilled, SqlCommand commandToRun )
    {
        ListToBeFilled = listToBeFilled;
        Command = commandToRun;
    }

    public void RunFor ( Func<SqlDataReader, object> rowProcessing )
    {
        ReadFromDb();
        ProcessRow(rowProcessing);
    }

    private void ReadFromDb ()
    {
        try
        {
            Reader = Command.ExecuteReader();
        }
        catch ( Exception e )
        {
            // handle the exception
        }
    }

    private void ProcessRow ( Func<SqlDataReader, object> rowProcessing )
    {
        if ( Reader.HasRows )
        {
            while ( Reader.Read() )
            {
                var result = rowProcessing(Reader);
                ListToBeFilled.Add(result);
            }
        }

        Reader.Close();
    }
}

And the callee:

var filler = new FillerFromDb(list/*or _Columns*/, _cmd);

filler.RunFor(row => row.GetValue(0));

If you are into refactoring, wrapping your worker functions in classes like this, you would feel better.

Answer 3

Try this:

private void CommonBlock(Action act)
    {
        // some code
        if (act!=null) act.Invoke();
        //next code
    }

to call with the specific code use:

CommonBlock(()=>{Debug.WriteLine("you action");/*Anything you want to do*/});

Answer 4

Simplest way is to create an extension method:

public static class Extensions
{
  public static IDataReader Read(this IDataReader reader, Action<IDataReader> action)
  {
    if(reader.HasRows)
    {
      while(reader.Read())
      {
        action(reader);
      }
    }
  }
}

An example, using your original sample, would be:

public List<object> TableList()
{
    List<object> list = new List<object>();

    _cmd.CommandText = "SELECT * FROM sys.tables";

    try
    {
        _read = _cmd.ExecuteReader();
    }
    catch (Exception e)
    {
                     //please ignore this try catch for now
        list[0] = e; //this is a temporary measure until I finalize error handling
    }

    _read.Read(r => list.Add(r.GetValue(0)));        
    _read.Close();

    return list;
}

Answer 5

Pass the differences into the function via an Action or Func. Write one common version and then just pass in what is different to be executed.