Run stored procedure from repository using EF

I need to dynamically call a stored procedures from my repository, and here is a my implementation:

public void ExecProcedure(string name, Dictionary param = null)
        {
            List sqlParametesList = new List();

            try
            {
                if (param != null)
                {
                    name = NormalizeProcedureName(name, param);
                    foreach (var item in param)
                    {
                        sqlParametesList.Add(new SqlParameter(item.Key, item.Value));
                    }
                }

                context.Database.ExecuteSqlCommand(name, sqlParametesList.ToArray());
            }
            catch (Exception e)
            {                
                throw new Exception(string.Format("Error in ExecProcedure: {0}", e.Message));
            }
        }

There is 3 case of call ExecProcedure():

• _repo.ExecProcedure("usp_Test"); // call without parameters

• _repo.ExecProcedure("usp_Test2 @id", param); // set parameter name after procedure name and pass value as param

-
_repo.ExecProcedure("usp_Test2", param); // if passing param then we use method NormalizeProcedureName() to set parameter name after procedure name

private string NormalizeProcedureName(string name, Dictionary param)
{
    if (!name.Contains("@"))
    {
        StringBuilder sb = new StringBuilder();
        string delimiter = "";
        sb.Append(name);

        foreach (var item in param)
        {                    
            sb.Append(delimiter);
            sb.Append(" @");
            sb.Append(item.Key);
            delimiter = ",";
        }

        name = sb.ToString();
    }
    return name;
}

What do you think about this implementations?

Any advice will be appreciated. Thanks

Dictionary param: I realize why you named it param and not params, but the real solution was to call it parameters.

Even worse as a name is sqlParametesList: it contains a typo, and the name also contains the type ("List") which is completely superfluous.

But the real problem is that a Dictionary is too limited. What if you need to pass an int or a datetime for instance? You even acknowledge this by converting the strings to objects, which isn't a solution at all.

NormalizeProcedureName is a seriously bad idea. It doesn't even do what it says, and what it does can be done far simpler with a string.join() -- not that this would be a good idea either.

I don't get the point of this:

throw new Exception(string.Format("Error in ExecProcedure: {0}", e.Message));

The stack trace should tell you where the exception happened, but you throw it away to create a far simpler error message.

IMO it would be far better to create a custom class to hold the necessary parameter data: the parameter name, its value and its type. You then pass a list of these objects to ExecProcedure, and loop through the list to construct the parameters you need to pass to ExecuteSqlCommand, making sure to also include the correct SqlDbType.

Or perhaps you can simply use SqlParameter instead of a custom class, it depends a bit on how you've abstracted your db logic from your business logic.

EDIT: To provide some guidance, here's an example of how I handled a similar problem.

I had to call a stored proc in an Oracle DB, which required a number of parameters. So I created this class:

public class LogMessageParameters
{
    public DateTime? Date { get; set; }

    public string From { get; set; }

    public long? Code { get; set; }

    public List ToOracleParameters()
    {
        return new List
        {
            new OracleParameter("DATE", OracleDbType.Date, Date, ParameterDirection.Input),
            new OracleParameter("FROM", OracleDbType.VarChar, From, ParameterDirection.Input),
            new OracleParameter("CODE", OracleDbType.Number, Code, ParameterDirection.Input),
            new OracleParameter("ID", OracleDbType.Number, ParameterDirection.Output)
        };
    }
}

Now all I needed to do was assign the proper values to the properties, and pass this class to the method where I needed to use the parameters.

Is this the ideal way? Not if I had to create a dozen of these classes, no: then I'd made sure to use reflection to figure out the type of a property so I could return the proper OracleDbType, I'd have used an attribute on a property to store the parameter name, etc.

StackExchange Code Review Q#104306, answer score: 2