2
\$\begingroup\$

Let me preface this with a few things before I get into my question:

  • I am making the leap from VBA/VB6 to .NET, (boy is it a big one), so I am noob to .NET best practices and so on.
  • I am re-writing en existing excel add-in (written in VBA) that has a library of subs and functions which are used to execute queries and return their results back to excel. I am modifying these functions to optionally return arrays, which means that I need to materialize a query's results in a .NET DataReader, then into a 2-D array with as little overhead as possible.
  • And yes, I am fully aware that there are several tried and tested ORM's out there that have various methods for mapping data to strongly typed objects (or even dynamics), but there are over 50 functions in this add-in (soon to be more), and the end goal is to return a 2-d array. It is not feasible to write 50 or more classes (or to use anonymous classes) to map the data into a generic list, only to then convert said list into an array.

Saying all of that, I am looking for improvements/suggestions for some methods that I have written to map a query in to a dynamic array. Async execution is on my radar, but I just don't yet have the knowledge on how I could leverage that to populate the array; however, if any one else does, I am all ears.

Class: ADOWrapper As per @JesseC.Slicer's request I have included my ADOWrapper class

public class ADOWrapper
{
    private System.Data.Common.DbProviderFactory dbProvider;
    private int commandTimeOut;
    private bool deriveParameters;


    //constructor
    public ADOWrapper(string providerName, int commandTimeOut = 0, bool deriveParameters = false)
    {
        this.dbProvider = DbProviderFactories.GetFactory(providerName);
        this.commandTimeOut = commandTimeOut;
        this.deriveParameters = deriveParameters;
    }

    //Method
    public IEnumerable<object[]> ExecuteQuery   (
                                                        System.Data.CommandType commandType,
                                                        string commandText, string connectionString,
                                                        params KeyValuePair<String, Object>[] parameterValues
                                                  )
    {


        using (IDbConnection connection = this.dbProvider.CreateConnection())
        {
            connection.ConnectionString = connectionString;
            connection.Open();

            IDbCommand command = connection.CreateCommand();

            command.CommandText = commandText;
            command.CommandType = commandType;
            command.CommandTimeout = this.commandTimeOut;

            if (!((parameterValues == null) || (parameterValues.Length == 0)))
                command = GetParameterizedCommand(command, parameterValues);

            using (IDataReader reader = command.ExecuteReader(System.Data.CommandBehavior.CloseConnection))
            {

                var indices = Enumerable.Range(0, reader.FieldCount).ToArray();
                foreach (IDataRecord record in reader as System.Collections.IEnumerable)
                    yield return indices.Select(i => record[i]).ToArray();

            }

        }

    }

    private IDbCommand GetParameterizedCommand(System.Data.IDbCommand command, params KeyValuePair<String, Object>[] parameterValues)
    {

        //can only derive parameters if it is a stored proc
        if (this.deriveParameters && command.CommandType == System.Data.CommandType.StoredProcedure)
        {

            DerivedParameters parameterMapper = new DerivedParameters();
            command = parameterMapper.AssignParameters(this.dbProvider, command, parameterValues);

        }
        else if (!this.deriveParameters)
        {

            AssummedParameters parameterMapper = new AssummedParameters();
            command = parameterMapper.AssignParameters(command, parameterValues);

        }
        else
        {
            throw new System.ArgumentException("Parameter cannot be of type 'Text' if deriveParameters is set to true", "commandType");
        }

        return command;

    }

}


internal class DerivedParameters
{
    public IDbCommand AssignParameters(DbProviderFactory dbProvider, IDbCommand command, params KeyValuePair<String, Object>[] parameterValues)
    {

        DbCommandBuilder commandBuilder = dbProvider.CreateCommandBuilder();
        //uses reflection to get method
        MethodInfo method = commandBuilder.GetType().GetMethod("DeriveParameters", BindingFlags.Public | BindingFlags.Static);
        method.Invoke(null, new object[] { command });

        if (command.Parameters.Count == 0)
            return command;

        IDbDataParameter defaultParam = (IDbDataParameter)command.Parameters[0];

        /*  
        skip first value in collection if there exists a defualt 
        return value 
        */
        int i = (defaultParam.ParameterName == "@RETURN_VALUE") ? 1 : 0;

        foreach (KeyValuePair<String, Object> paramKVP in parameterValues)
        {

            IDbDataParameter param = (IDbDataParameter)command.Parameters[i];

            if (paramKVP.Key != param.ParameterName) 
                throw new System.ArgumentException("The specified parameter does not have a"
                                                    + "corresponding match in the database", 
                                                    paramKVP.Key); 

            param = SetCommonProperties(param, paramKVP.Value.ToString());
            param.Value = paramKVP.Value ?? System.DBNull.Value;
            i++;

        }

        return command;

    }

    private IDbDataParameter SetCommonProperties(IDbDataParameter parameter, string parameterValue)
    {

        switch (parameter.DbType)
        {
            case DbType.Single:
                if (Single.TryParse(parameterValue, out Single singleValue))
                {
                    parameter.Precision = NumericDataAttributes.CalculatePrecision(singleValue);
                    parameter.Scale = NumericDataAttributes.CalculateNumericScale(singleValue);
                }
                break;

            case DbType.Double:
                if (double.TryParse(parameterValue, out double doubleValue))
                {
                    parameter.Precision = NumericDataAttributes.CalculatePrecision(doubleValue);
                    parameter.Scale = NumericDataAttributes.CalculateNumericScale(doubleValue);
                }
                break;

            case DbType.Decimal:
                if (decimal.TryParse(parameterValue, out decimal decimalValue))
                {
                    parameter.Precision = NumericDataAttributes.CalculatePrecision(decimalValue);
                    parameter.Scale = NumericDataAttributes.CalculateNumericScale(decimalValue);
                }
                break;

            case DbType.String:
                parameter.Size = parameterValue.Length;
                break;
        };

        return parameter;

    }

}


internal class AssummedParameters
{
    public IDbCommand AssignParameters(IDbCommand command, params KeyValuePair<String, Object>[] parameterValues)
    {

        foreach (KeyValuePair<String, Object> paramKVP in parameterValues)
        {

            IDbDataParameter param = command.CreateParameter();

            param.ParameterName = paramKVP.Key;
            param = SetCommonProperties(param, paramKVP.Value.ToString());
            param.Value = paramKVP.Value ?? System.DBNull.Value;
            param.Direction = ParameterDirection.Input;

            command.Parameters.Add(param);

        }

        return command;

    }


    private IDbDataParameter SetCommonProperties(IDbDataParameter parameter, string parameterValue)
    {

        if (byte.TryParse(parameterValue, out byte byteValue))
            parameter.DbType = DbType.Byte;

        else if (sbyte.TryParse(parameterValue, out sbyte sbyteValue))
            parameter.DbType = DbType.SByte;

        else if (Int16.TryParse(parameterValue, out Int16 smallintValue))
            parameter.DbType = DbType.Int16;

        else if (Int32.TryParse(parameterValue, out Int32 intValue))
            parameter.DbType = DbType.Int32;

        else if (Int64.TryParse(parameterValue, out Int64 bigintValue))
            parameter.DbType = DbType.Int64;

        else if (Single.TryParse(parameterValue, out Single singleValue))
        {
            parameter.DbType = DbType.Single;
            parameter.Precision = NumericDataAttributes.CalculatePrecision(singleValue);
            parameter.Scale = NumericDataAttributes.CalculateNumericScale(singleValue);
        }

        else if (double.TryParse(parameterValue, out double doubleValue))
        {
            parameter.DbType = DbType.Double;
            parameter.Precision = NumericDataAttributes.CalculatePrecision(doubleValue);
            parameter.Scale = NumericDataAttributes.CalculateNumericScale(doubleValue);
        }

        else if (decimal.TryParse(parameterValue, out decimal decimalValue))
        {
            parameter.DbType = DbType.Decimal;
            parameter.Precision = NumericDataAttributes.CalculatePrecision(decimalValue);
            parameter.Scale = NumericDataAttributes.CalculateNumericScale(decimalValue);
        }

        else if (DateTime.TryParse(parameterValue, out DateTime dateValue))
            parameter.DbType = DbType.DateTime;

        else if (bool.TryParse(parameterValue, out bool boolValue))
            parameter.DbType = DbType.Boolean;

        else if (parameter.DbType == DbType.String)
            parameter.Size = parameterValue.Length;

        return parameter;

    }


}


internal class NumericDataAttributes
{

    //2 overload methods (makes for a total of 3 per each of CalculatePrecision and CalculateNumericScale)
    public static byte CalculatePrecision(Single valueIn)
    {

        return (byte)valueIn.ToString(CultureInfo.InvariantCulture).Replace(".", "").Length;

    }

    public static byte CalculateNumericScale(Single valueIn)
    {

        return (byte)valueIn.ToString(CultureInfo.InvariantCulture).Split('.')[1].Length;

    }



    public static byte CalculatePrecision(double valueIn)
    {

        return (byte)valueIn.ToString(CultureInfo.InvariantCulture).Replace(".", "").Length;

    }

    public static byte CalculateNumericScale(double valueIn)
    {

        return (byte)valueIn.ToString(CultureInfo.InvariantCulture).Split('.')[1].Length;

    }



    public static byte CalculatePrecision(decimal valueIn)
    {

        return (byte)valueIn.ToString(CultureInfo.InvariantCulture).Replace(".", "").Length;

    }

    public static byte CalculateNumericScale(decimal valueIn)
    {

        return (byte)valueIn.ToString(CultureInfo.InvariantCulture).Split('.')[1].Length;

    }

}

}

Usage:

static void Main(string[] args)
{


    string connString = "Persist Security Info=False;User ID=***;Password=***;Initial Catalog=***;Server=*****";

    ADOWrapper SQLDataAdapter = new ADOWrapper("System.Data.SqlClient", 0, false);

    var queryResult = SQLDataAdapter.ExecuteQuery(CommandType.Text, "Select * From TABLE_NAME", 
                                                  connString).ToArray();

    object[,] twoDArray = new object[queryResult.Length, queryResult.Max(x => x.Length)];

    Get2dArrayFromJagged(queryResult, twoDArray);

    Print2DArray(twoDArray); 

    Console.ReadLine();

}

public static void Get2dArrayFromJagged(object[][] jaggedArray, object[,] returnArray)
{

    for (int i = 0; i < jaggedArray.Length; i++)
    {
        for (int j = 0; j < jaggedArray[i].Length; j++)
        {
            returnArray[i, j] = jaggedArray[i][j];
        }
    }
}


public static void Print2DArray<T>(T[,] matrix)
{
    for (int i = 0; i < matrix.GetLength(0); i++)
    {
        for (int j = 0; j < matrix.GetLength(1); j++)
        {
            Console.Write("|" + matrix[i, j] + "|" + "\t");
        }
        Console.WriteLine();
    }
}

}
\$\endgroup\$
  • 1
    \$\begingroup\$ IDbCommand inherits from IDisposable and should also be wrapped in a using construct. \$\endgroup\$ – Jesse C. Slicer Oct 24 at 21:57
  • \$\begingroup\$ @JesseC.Slicer indeed. I probably could go without setting the connection behavior as well , b/c ‘IDataReader’ also inherits from ‘IDisposible’ \$\endgroup\$ – rickmanalexander Oct 24 at 22:40
  • \$\begingroup\$ @JesseC.Slicer Actually, I take that back. Wrapping IDbCommand in a using construct yields the "cannot assign to to 'command' because it is a 'using variable'" error, b/c I have command = GetParameterizedCommand(command, parameterValues); . As a work around, I suppose that I could use try/catch for IDbCommand and call Dispose() explicitly at the bottom of the try block and in the catch block. But the idea of having to do that makes me think that I should restructure the method so that I could utilize the using statement. \$\endgroup\$ – rickmanalexander Oct 25 at 11:40
  • \$\begingroup\$ Cam you add the code for GetParameterizedCommand? I feel there's some deeper issues now. \$\endgroup\$ – Jesse C. Slicer Oct 25 at 14:47
  • \$\begingroup\$ @JesseC.Slicer I'll do you one better and post the whole class, because it's relevant. I haven't implemented all the methods and overloads that I plan to yet because I am still very much in the development stage. Saying that, the whole using KeyValuePair<string,object> within a param array will be relpaced by a generic dictionary<string, object> w/o using param. \$\endgroup\$ – rickmanalexander Oct 25 at 16:46
2
\$\begingroup\$

The current

foreach (IDataRecord record in reader as System.Collections.IEnumerable)...

seem complicated for no particular reason and is making assumptions about implementation concerns.

With all that effort to make the function abstract I would suggest keep the reader traversal simple.

//...

using (IDataReader reader = command.ExecuteReader()) {
    var indices = Enumerable.Range(0, reader.FieldCount).ToArray();
    while (reader.Read()) {
        yield return indices.Select(i => reader[i]).ToArray();
    }
}

//...

Parameter values check condition can be simplified using a null conditional to

//...

if (parameterValues?.Length > 0)
    //...

//...

If parameterValues is null the condition will default to false

command should be wrapped in a using statement but cannot because of how it is passed and reassign in the supporting methods.

Since all those method only modify the variable, they should be refactored to reflect that

private void SetCommandParameters(IDbCommand command, params KeyValuePair<String, Object>[] parameterValues) {
    //can only derive parameters if it is a stored proc
    if (this.deriveParameters && command.CommandType == System.Data.CommandType.StoredProcedure) {

        DerivedParameters parameterMapper = new DerivedParameters();
        parameterMapper.AssignParameters(this.dbProvider, command, parameterValues);

    } else if (!this.deriveParameters) {

        AssummedParameters parameterMapper = new AssummedParameters();
        parameterMapper.AssignParameters(command, parameterValues);

    } else {
        throw new System.ArgumentException("Parameter cannot be of type 'Text' if deriveParameters is set to true", "commandType");
    }
}

Note the name change.

The ExecuteQuery becomes

public IEnumerable<object[]> ExecuteQuery(CommandType commandType, string commandText, string connectionString, params KeyValuePair<String, Object>[] parameterValues) {
    using (IDbConnection connection = this.dbProvider.CreateConnection()) {
        connection.ConnectionString = connectionString;
        connection.Open();
        using (IDbCommand command = connection.CreateCommand()) {
            command.CommandText = commandText;
            command.CommandType = commandType;
            command.CommandTimeout = this.commandTimeOut;

            if (parameterValues?.Length > 0)
                SetCommandParameters(command, parameterValues);

            using (IDataReader reader = command.ExecuteReader()) {
                var indices = Enumerable.Range(0, reader.FieldCount).ToArray();
                while (reader.Read()) {
                    yield return indices.Select(i => reader[i]).ToArray();
                }
            }
        }
    }
}

While I applaud the effort to make the code abstract, it still tightly couples itself to implementation concerns.

Based on a review of the current implementation's dependency on DbProviderFactory, the following abstraction and default implementation was derived.

public interface IDbProviderFactory {
    IDbConnection CreateConnection(string connectionString);
    DbCommandBuilder CreateCommandBuilder();
}

public class DefaultDbProviderFactory : IDbProviderFactory {
    private readonly DbProviderFactory dbProvider;

    public DefaultDbProviderFactory(string providerName) {
        dbProvider = DbProviderFactories.GetFactory(providerName);
    }

    public DbCommandBuilder CreateCommandBuilder() => dbProvider.CreateCommandBuilder();

    public IDbConnection CreateConnection(string connectionString) {
        var connection = dbProvider.CreateConnection();
        connection.ConnectionString = connectionString;
        return connection;
    }
}

Along with an aggregation of the run-time dependencies into a POCO.

public class ADOWrapperOptions {
    public string ConnectionString { get; set; }
    public int CommandTimeOut { get; set; } = 0;
    public bool DeriveParameters { get; set; } = false;
}

With the parameter helpers also refactored accordingly

DerivedParameters

internal class DerivedParameters {
    public void AssignParameters(IDbProviderFactory dbProvider, IDbCommand command, KeyValuePair<String, Object>[] parameterValues) {

        DbCommandBuilder commandBuilder = dbProvider.CreateCommandBuilder();
        //uses reflection to get method
        MethodInfo method = commandBuilder.GetType().GetMethod("DeriveParameters", BindingFlags.Public | BindingFlags.Static);
        method.Invoke(null, new object[] { command });

        if (command.Parameters.Count == 0)
            return;

        IDbDataParameter defaultParam = (IDbDataParameter)command.Parameters[0];

        /*  
        skip first value in collection if there exists a defualt 
        return value 
        */
        int i = (defaultParam.ParameterName == "@RETURN_VALUE") ? 1 : 0;

        foreach (KeyValuePair<String, Object> paramKVP in parameterValues) {

            IDbDataParameter param = (IDbDataParameter)command.Parameters[i];

            if (paramKVP.Key != param.ParameterName)
                throw new System.ArgumentException("The specified parameter does not have a"
                                                    + "corresponding match in the database",
                                                    paramKVP.Key);

            SetCommonProperties(param, paramKVP.Value.ToString());
            param.Value = paramKVP.Value ?? System.DBNull.Value;
            i++;
        }
    }

    //... omitted for brevity
}

AssummedParameters

internal class AssummedParameters {
    public void AssignParameters(IDbCommand command, KeyValuePair<String, Object>[] parameterValues) {

        foreach (KeyValuePair<String, Object> paramKVP in parameterValues) {
            IDbDataParameter param = command.CreateParameter();
            param.ParameterName = paramKVP.Key;
            param.Value = paramKVP.Value ?? System.DBNull.Value;
            param.Direction = ParameterDirection.Input;

            SetCommonProperties(param, paramKVP.Value.ToString());

            command.Parameters.Add(param);
        }
    }

    //...omitted for brevity
}

There is potential to create some form of strategy abstraction in a future refactor

After refactoring, the wrapper becomes

public class ADOWrapper {
    private readonly IDbProviderFactory dbProvider;
    private readonly int commandTimeOut;
    private readonly bool deriveParameters;
    private readonly string connectionString;

    //constructor
    public ADOWrapper(IDbProviderFactory dbProvider, ADOWrapperOptions options) {
        this.dbProvider = dbProvider;
        commandTimeOut = options.CommandTimeOut;
        deriveParameters = options.DeriveParameters;
        connectionString = options.ConnectionString;
    }

    //Method
    public IEnumerable<object[]> ExecuteQuery(CommandType commandType, string commandText, params KeyValuePair<String, Object>[] parameterValues) {
        using (IDbConnection connection = dbProvider.CreateConnection(connectionString)) {
            connection.Open();
            using (IDbCommand command = connection.CreateCommand()) {
                command.CommandText = commandText;
                command.CommandType = commandType;
                command.CommandTimeout = commandTimeOut;

                if (parameterValues?.Length > 0)
                    SetCommandParameters(command, parameterValues);

                using (IDataReader reader = command.ExecuteReader()) {
                    var indices = Enumerable.Range(0, reader.FieldCount).ToArray();
                    while (reader.Read()) {
                        yield return indices.Select(i => reader[i]).ToArray();
                    }
                }
            }
        }
    }

    private void SetCommandParameters(IDbCommand command, params KeyValuePair<String, Object>[] parameterValues) {
        //can only derive parameters if it is a stored proc
        if (this.deriveParameters && command.CommandType == System.Data.CommandType.StoredProcedure) {
            DerivedParameters parameterMapper = new DerivedParameters();
            parameterMapper.AssignParameters(this.dbProvider, command, parameterValues);
        } else if (!this.deriveParameters) {
            AssummedParameters parameterMapper = new AssummedParameters();
            parameterMapper.AssignParameters(command, parameterValues);
        } else {
            throw new System.ArgumentException("Parameter cannot be of type 'Text' if deriveParameters is set to true", "commandType");
        }
    }
}

The example usage now look like the following

string connString = "Persist Security Info=False;User ID=***;Password=***;Initial Catalog=***;Server=*****";

IDbProviderFactory provider = new DefaultDbProviderFactory("System.Data.SqlClient");

ADOWrapperOptions options = new ADOWrapperOptions() {
    ConnectionString = connString
};

ADOWrapper SQLDataAdapter = new ADOWrapper(provider, options);

var queryResult = SQLDataAdapter.ExecuteQuery(CommandType.Text, "Select * From TABLE_NAME").ToArray();

object[,] twoDArray = new object[queryResult.Length, queryResult.Max(x => x.Length)];

Get2dArrayFromJagged(queryResult, twoDArray);

//...

And having the abstracted dependency allows the wrapper the flexibility to be tested in isolation.

Simple example

[TestClass]
public class ADOWrapperTests {
    [TestMethod]
    public void ExecuteQuery_Should_Return_One_Row_Two_Columns() {
        //Arrange - Using Moq
        var reader = new Mock<IDataReader>();
        reader.Setup(_ => _.FieldCount).Returns(2);
        reader.Setup(_ => _[0]).Returns("Hello World");
        reader.Setup(_ => _[1]).Returns("I am working");
        reader.SetupSequence(_ => _.Read())
            .Returns(true)
            .Returns(false);

        var provider = Mock.Of<IDbProviderFactory>(dbpf =>
            dbpf.CreateConnection(It.IsAny<string>()) == Mock.Of<IDbConnection>(c =>
                c.CreateCommand() == Mock.Of<IDbCommand>(cmd =>
                    cmd.ExecuteReader() == reader.Object
                )
            )
        );

        ADOWrapperOptions options = new ADOWrapperOptions() {
            ConnectionString = "Fake connectino string"
        };

        ADOWrapper SQLDataAdapter = new ADOWrapper(provider, options);

        //Act
        var queryResult = SQLDataAdapter.ExecuteQuery(CommandType.Text, "Select * From TABLE_NAME").ToArray();
        object[,] twoDArray = new object[queryResult.Length, queryResult.Max(x => x.Length)];
        Get2dArrayFromJagged(queryResult, twoDArray);

        //Assert - FluentAssertions
        twoDArray.Should().HaveCount(2);            
    }

    public static void Get2dArrayFromJagged(object[][] jaggedArray, object[,] returnArray) {

        for (int i = 0; i < jaggedArray.Length; i++) {
            for (int j = 0; j < jaggedArray[i].Length; j++) {
                returnArray[i, j] = jaggedArray[i][j];
            }
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ This is the kind of answer that I was looking for and I really appreciate the time and effort that you put into this. Creating the correct level of abstraction is what I was really struggling with, especially being a newbie to .NET. One final question: Is there any way to materialize the result of a query into a 2-D Object Array that is faster than the current method I am using?? By that I mean, can I skip the conversion of a jagged array to a 2-D array and instead return the 2-D array directly from Execute Query? \$\endgroup\$ – rickmanalexander Oct 27 at 17:56
  • \$\begingroup\$ You used function is efficient enough. You can convert that to an extension method method and use it as a helper \$\endgroup\$ – Nkosi Oct 27 at 21:31

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.