I have a web application that communicates with SQL server. Rather than hard-coding all of the query strings, I have opted to store them in a global resource file. Is that considered bad practice?
On a side note, when I do this, Visual Studio yells at me about the possibility of SQL injection, despite those queries being parameterized (not to mention the "spelling" warnings inside the resource file).
Practices fall into a range (e.g. Avoid, Prefer, Use, etc.) and depend on context.
If you have a mandate from on high that stored-procs shalt not be used and neither shall ye use an ORM, then storing your complex SQL as a resource is not that bad of a practice because you at least don't have to escape characters in a System.String and you at least keep it somewhat safe from eyes. If your SQL is dynamic in nature, combining resource files with a text templating mechanism is fairly clean.
That said, generally (i.e. it seems in most contexts) using resource files should be avoided unless there's a clear benefit in maintenance costs, readability, and capability. There are quite a few clean ways to bind stored procedures to code; there are a number of competent ORM tools and mini-data access layers (aka micro-ORMs in today's parlance) that might do a better job.
Having the SQL queries separated from the application code is a good thing. Stored procedures is the normal way to do this, but if that's not feasible and you have to use SQL directly I think your approach is good. With recent versions of SQL server parameterized queries are precompiled the first time they are run and give similar performance to an SP.
I would however advise you to look into other data access methods such as linq-to-sql which automates the SQL query generation and gives you a cleaner interface in the code.
Resource files are definitley not the place for this. The point of resource files is to be a centralized repository for things that can be localized (i.e. overridden by other resource files that are defined for other languages/cultures).
For example, you'd put a string "Hello" in a X.resources.dll, but then you could also create a es-ES\X.resources.dll for Spanish, in which the string would say "Hola" instead — then when your application queries for the string, it will get whatever version matches the language/culture of the user's operating system configuration.
If you want your SQL to be easily changed without recompilation of code, put it in your App.config and use the ConfigurationManager class to read it out. If you don't want it to be changeable without code recompilation, just hard code the thing as a static/const string. That said, the ideal, of course, is to make real stored procedures.
I think many people consider hard coded SQL to be a bad practice regardless of how it is stored... :-)
I'm going to assume that there is some compelling reason for not using Linq to SQL, or Entity Framework, or another ORM tool?
If you must use hard coded SQL in your application, would argue that it is BETTER inline in your code because it makes your code more readable, and therefor more maintainable...
I've built an application using this pattern - in PHP, not .Net, but the principles are the same.
Benefits:
Drawbacks:
In my experience, the benefits don't stack up against the drawbacks.
BTW - many of those drawbacks also apply to stored procedures. There's a good case for using stored procs - but there's an equally good case for not using them.
I don't see anything particularly "bad" with doing this. It really isn't much different than hard coding the sql code within your code, and only minorly different than generating the SQL ad-hoc at runtime.
You say that you are using parameterized queries, so you shouldn't have to worry about script injection.
If you are storing the sql in a resource file to adhere to the DRY principle, then you may want to use some kind of DAL for that purpose instead. Like Entity Framework (EF) or Linq-to-SQL
It is not necessarily bad practice, but it makes it harder to read you program if one needs to open another file and find the right key.
Visual Studio complains because it cannot see that the value is constant and that it always comes from a trusted source.
Having SQL in source files is no more "hard-coding" than having the rest of the program code in the source files. Why are you doing this in the first place? To re-use queries? Maybe you should consider store procedures instead...
The thing that comes to mind is that this is simply adding unnecessary complications to the code.
Yes, you could keep the queries in a resource file. Or, to insure that no one messes with them once the project is deployed, you could encrypt them and store them in a file. Or even better yet, you could encrypt them and then store them on a database so that no one who had access to the machine could mess with them.
But eventually, I have to ask, what's the point? Just hard code them and be done with it.
KISS.
you could write store procedures and call them in your code rather than read the queries stored somewhere outside.
