HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Proper way to work with stored procedures in Entity Framework Core

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
storedprocedurescorewithwayworkproperframeworkentity

Problem

I need to work with stored procedures, with the help of Entity Framework Core. This is my function:

public async Task CheckIfUserRegistered(string phoneNumber, DateTime dateOfBirth)
    {
        if (string.IsNullOrWhiteSpace(phoneNumber))
        {
            return false;
        }
        using (var cmd = _dbContext.Database.GetDbConnection().CreateCommand())
        {
            cmd.CommandText = "dbo.CheckIfUserRegistered";
            cmd.CommandType = CommandType.StoredProcedure;

            cmd.Parameters.Add(new SqlParameter("@phoneNumber", SqlDbType.NVarChar) { Value = phoneNumber });
            cmd.Parameters.Add(new SqlParameter("@dateOfBirth", SqlDbType.Date) { Value = dateOfBirth });

            cmd.Parameters.Add(new SqlParameter("@registered", SqlDbType.Bit) { Direction = ParameterDirection.Output });

            if (cmd.Connection.State != ConnectionState.Open)
            {
                cmd.Connection.Open();
            }

            await cmd.ExecuteNonQueryAsync();

            return (bool)cmd.Parameters["@registered"].Value;
        }
    }


I'm not sure, with the correctness of this function, is this the right way to work with stored procedures in EF Core? Don't I have problems with forgotten connections, memory leakings, etc?

Solution

The biggest thing I would change is to return the connection to the state you found it.

bool isOpen = cmd.Connection.State == ConnectionState.Open;
if (!isOpen)
{
    cmd.Connection.Open();
}

await cmd.ExecuteNonQueryAsync();

if (!isOpen)
{
    cmd.Connection.Close();
}


Then, from there, I would create a helper method to build the SqlParameter objets for you:

private SqlParameter BuildParamter(string name, SqlDbType type, object value, ParameterDirection? direction)
{
    var parameter = new SqlParameter(name, type);

    if (value != null)
    {
        parameter.Value = value;
    }

    if (direction.HasValue)
    {
        parameter.Direction = direction.Value;
    }

    return parameter;
}


Then your parameters go from:

cmd.Parameters.Add(new SqlParameter("@phoneNumber", SqlDbType.NVarChar) { Value = phoneNumber });
cmd.Parameters.Add(new SqlParameter("@dateOfBirth", SqlDbType.Date) { Value = dateOfBirth });

cmd.Parameters.Add(new SqlParameter("@registered", SqlDbType.Bit) { Direction = ParameterDirection.Output });


To:

cmd.Parameters.Add(BuildParameter("@phoneNumber", SqlDbType.NVarChar, phoneNumber));
cmd.Parameters.Add(BuildParameter("@dateOfBirth", SqlDbType.Date, dateOfBirth));

cmd.Parameters.Add(BuildParameter("@registered", SqlDbType.Bit, null, ParameterDirection.Output));


It's less verbose, but if you build a lot of SqlParameter objects it might be handy to have one method you can use for all of them. (You can optionally add a int? size parameter to the method as well.)

Then, extract a SetStoredProcedure method which would take the input SqlCommand and string, set the type and command text to "dbo." + name, then you add the parameters, then ExecuteStoredProcedure would do the connection checking and then you can return your cmd.Parameters["@registered"].Value as you are. In the end something like:

SetStoredProcedure("CheckIfUserRegistered", cmd):

cmd.Parameters.Add(BuildParameter("@phoneNumber", SqlDbType.NVarChar, phoneNumber));
cmd.Parameters.Add(BuildParameter("@dateOfBirth", SqlDbType.Date, dateOfBirth));

cmd.Parameters.Add(BuildParameter("@registered", SqlDbType.Bit, null, ParameterDirection.Output));

ExecuteStoredProcedure(cmd);

return (bool)cmd.Parameters["@registered"].Value;


Optionally with your async/await pattern.

This way, if you create more stored procedures it's less work to make them operate.

Code Snippets

bool isOpen = cmd.Connection.State == ConnectionState.Open;
if (!isOpen)
{
    cmd.Connection.Open();
}

await cmd.ExecuteNonQueryAsync();

if (!isOpen)
{
    cmd.Connection.Close();
}
private SqlParameter BuildParamter(string name, SqlDbType type, object value, ParameterDirection? direction)
{
    var parameter = new SqlParameter(name, type);

    if (value != null)
    {
        parameter.Value = value;
    }

    if (direction.HasValue)
    {
        parameter.Direction = direction.Value;
    }

    return parameter;
}
cmd.Parameters.Add(new SqlParameter("@phoneNumber", SqlDbType.NVarChar) { Value = phoneNumber });
cmd.Parameters.Add(new SqlParameter("@dateOfBirth", SqlDbType.Date) { Value = dateOfBirth });

cmd.Parameters.Add(new SqlParameter("@registered", SqlDbType.Bit) { Direction = ParameterDirection.Output });
cmd.Parameters.Add(BuildParameter("@phoneNumber", SqlDbType.NVarChar, phoneNumber));
cmd.Parameters.Add(BuildParameter("@dateOfBirth", SqlDbType.Date, dateOfBirth));

cmd.Parameters.Add(BuildParameter("@registered", SqlDbType.Bit, null, ParameterDirection.Output));
SetStoredProcedure("CheckIfUserRegistered", cmd):

cmd.Parameters.Add(BuildParameter("@phoneNumber", SqlDbType.NVarChar, phoneNumber));
cmd.Parameters.Add(BuildParameter("@dateOfBirth", SqlDbType.Date, dateOfBirth));

cmd.Parameters.Add(BuildParameter("@registered", SqlDbType.Bit, null, ParameterDirection.Output));

ExecuteStoredProcedure(cmd);

return (bool)cmd.Parameters["@registered"].Value;

Context

StackExchange Code Review Q#148732, answer score: 5

Revisions (0)

No revisions yet.