CODEWITHSUNDEEP
c#parameters
majk86
majk86

Reputation: 125

Reuse method parameters

I have this code:

public int Delete(int id)
{
    string query = "DELETE FROM [table] WHERE [id] = @id";

    using (SqlConnection sqlConnection = new SqlConnection(connectionString))
    using (SqlCommand sqlCommand = new SqlCommand(query, sqlConnection))
    {
        sqlCommand.Parameters.Add("id", SqlDbType.Int).Value = id;

        sqlConnection.Open();

        if ((id = sqlCommand.ExecuteNonQuery()) == 0)
            throw new Exception("Not found.");

        return id;
    }
}

Is it OK to reuse parameter ID to store number of rows affected or should I rather declare new variable? What's generally better?

Upvotes: 2

Views: 741

Answers (3)

Heinzi
Heinzi

Reputation: 172200

Developers spend much more time reading code than writing code. Thus, one of your primary objectives should be to write to code that is easy to read and easy to understand.

If I have to read your code and just skim over the last line, I get the (false) impression that your method returns some kind of identifier. It doesn't. It returns the number of rows deleted.

Using a different variable makes your code easier to read and does not have any performance penalties.

    ...
    var rowsDeleted = sqlCommand.ExecuteNonQuery();
    if (rowsDeleted == 0)
        throw new Exception("Not found.");

    return rowsDeleted;

It also makes your code more robust to changes: Imagine that you some day decide to use GUIDs instead of integer IDs, so you change the type of your parameter to Guid. What happens? A completely unrelated part of your code breaks and needs to be fixed.

Upvotes: 5

Tim Schmelter
Tim Schmelter

Reputation: 460018

You should not ask what is more performant but what is more readable and therefore less error-prone.

The parameter is the ID you want to delete. ExecuteNonQuery returns the number of affected records. These are two different meanings, so you should use two different variables.

int numDeleted = sqlCommand.ExecuteNonQuery();
if(numDeleted == 0)
    throw new Exception("Could not delete record with id " + id);
else if(numDeleted > 1)
    throw new Exception(numDeleted + " records deleted with id " + id);

return numDeleted;

Upvotes: 4

BRAHIM Kamel
BRAHIM Kamel

Reputation: 13755

Is it OK to reuse parameter ID to store number of rows affected or should I rather declare new variable?

Yes it's ok to use the same variable since int is a value type and no other instruction depends on initial value. But, as Heinzi pointed out to make your code more readable and maintainable it will be better to define a new one

Upvotes: 1

Related Questions