CODEWITHSUNDEEP
c#sql-server
beginnerlaravelvue
beginnerlaravelvue

Reputation: 157

Is it safe to write dBCommand.AddParameter even though I'm not going to use it in a query?

Scenario: my query variable is dynamic, there are 4 possible values for that depending on the report type (_reportType). Meaning there are 4 different queries and some of it doesn't have @STAFF in the where condition, so my question is, is it safe to just leave my 

dBCommand.AddParameter("@STAFF", staff)

there or should I include if else condition just to be safe?

Like this

if(_reportType == 1) 
{
    dBCommand.AddParameter("@STAFF", staff);
}
else if (_reportType == 2) 
{
    //code
}
else if (_reportType == 3) 
{
    //code
}
else
{
     //Don't add dBCommand.AddParameter("@STAFF", staff);
}

Is it safe just to leave addParameter("@STAFF", staff) even though I'm not going to use it in a query?

Example I'm going to write

dBCommand.Initialize(string.Format(query, "RetailTable"), batch);
dBCommand.AddParameter("@STAFF", staff);

But the query value doesn't have @STAFF in the WHERE condition

Upvotes: 0

Views: 178

Answers (2)

dj079
dj079

Reputation: 1389

There are 2 glaring bad practices in your approach:

1. Generating dynamic query within the code.

This approach has many drawbacks and possible security loopholes. You should almost always avoid doing that.

Please go through the following links to understand this more:

https://codingsight.com/dynamic-sql-vs-stored-procedure/ https://www.ecanarys.com/Blogs/ArticleID/112/SQL-injection-attack-and-prevention-using-stored-procedure

2. Trying to use generic Where Clause that fits all your variations.

This approach is disaster in waiting, regardless of the query being written in your application code OR in a Stored Procedure.

This is an ugly code-smell and a maintenance nightmare.

No developer can ever be 100% sure that there will not be any change required during the lifespan of the application due to a simple fact that the client WILL need enhancements on regular bases.

So, even if this approach may work for you for a small period of time, this will blow back.

Assume, over the period, there are few more filter parameters added due to new requirements. Now, imagine how your code would look like and the possibilities it creates of problems you may get if they are not handled properly. Specially when YOU are not making those changes. Scary, right?

Always write code that will not only be easier to read and understand, but also easy to enhance and maintain, regardless of the person writing the code.

So, IMHO, you should add those if-else conditions OR use switch-case blocks to safeguard yourself and your client. It may look overkill in the start, but will surely payoff in future.

Hope this help!

Upvotes: 1

Dan Guzman
Dan Guzman

Reputation: 46203

It should generally be ok to specify unused parameters, aside from the minor overhead of sending the value to the server. The exception is if you execute DDL queries that have a restriction of being the only statement in the batch (e.g. CREATE VIEW). Those would fail due to the parameter.

Upvotes: 2

Related Questions