Parameterized SQL!
-
I can't figure out how to do a proper syntax-highlighted code block, so whatever:
private SqlCommand BuildDoThingCommand(SqlConnection conn) { SqlCommand DoThingCommand = conn.CreateCommand(); DoThingCommand.CommandTimeout = ConfigHelper.GetAppSetting<int>("CommandTimeout"); DoThingCommand.CommandType = CommandType.StoredProcedure; DoThingCommand.CommandText = ConfigHelper.GetAppSetting("DoThing"); foreach (string parameter in ConfigHelper.GetAppSetting("DoThingParameters").Split(';')) { string[] paramParts = parameter.Split(','); if (paramParts.Length == 2) DoThingCommand.Parameters.Add(paramParts[0], (SqlDbType)Enum.Parse(typeof(SqlDbType), paramParts[1])); else if (paramParts.Length == 3) DoThingCommand.Parameters.Add(paramParts[0], (SqlDbType)Enum.Parse(typeof(SqlDbType), paramParts[1]), int.Parse(paramParts[2])); else throw new Exception("Invalid parameter configuration for 'DoThingParameters'"); } return DoThingCommand;
Because there's ZERO chance that could ever go wrong. Protip: It went wrong. That's how I found it.
-
I can't figure out how to do a proper syntax-highlighted code block
triple backtick.
What's the point of doing a parametrized query, if the SQL and the parameters come from the same source?
-
Note it's only getting the names and types of the parameters. The values come elsewhere. And are hardcoded to array positions. So the "configurable" parameters gain nothing.
-
Hmm, it calls a stored procedure with name and arguments defined in settings... Sorry I don't see the WTF. I can imagine a situation where this could be useful.
EDIT: OK, if values are hardcoded positionally, then this is not needed. But you didn't show that part.
-
The place where the procedure was used did something like this:
cmd.Parameters[0].Value = ThingA
cmd.Parameters[1].Value = ThingB
cmd.Parameters[2].Value = ThingCI don't think any sane universe exists in which parameters will occur in a fixed order, but with variable names and types, and with varying procedure names.
Also, the number of parameters it was trying to redefine mismatched the number in the config.
-
Yeah if this part was also configurable, then it might have made sense.
It's a WTF.
-
Fuck that? How stupid? What kind of moron made that stupid pattern?
This reminds me of certain pattern my team decided to use:
function login(params){ doLogin(params.username, params.password); }
Funny that
three-back-ticks
works for code blocks, butthree-back-ticks + space
breaks the whole thing
-
I see code like this, and I don't even analyze the lines. I can just look at the block as a whole, and see that it's bad. SQL injection, inner-platform effect, blonde, brunette, redhead...