Which one is better between these two approaches?

[Guest]

Hi, I'm doing an application using C# and I have this question:

I have a method called sqlQueryBD which receives a string sql query and
executes it against a database.
I also have a class called sqlCompat which has one property "sqlText" with
its getter and setter methods. The purpose of this class to translate the
receiving sql query into an "escaped" sql query, for instance:
insert into people values ('cod001','Carl
Johnson','\\computer\directory\file01.txt')
is translated into:
insert into people values ('cod001','Carl
Johnson','\\\\computer\\directory\\file01.txt')

The constructor receiving a string parameter is as follows:
public sqlCompat(string text)
{
this.text = sqlCompatSTR(text.Trim());
}

These are getter and setter methods for sqlText:
private string text;
public string sqlText
{
get
{
return text;
}
set
{
text = sqlCompatSTR(value.Trim());
}
}
where sqlCompatSTR is in charge of doing the translation.

In a form called frmQuerySFG in a button event I have this and it works
perfectly:

sqlQueryBD(new sqlCompat("select * from table").sqlText);

But if I do this it works exactly the same way:

//Declare a frmQuerySFG attribute (this is outside of button's event)
sqlCompat s_sql = new sqlCompat();

//This goes inside the button event
s_sql.sqlText = "select * from table";
sqlQueryBD(s_sql.sqlText);

Ok, which one is better between these two approaches?
The first approach is faster to code because it is only one line of code,
but I don't know if there are disadvantages in creating a new instance each
time I call that method.
The second approach doesn't create a new instance each time sqlQueryBD is
called but it involves more lines of code.

I've been using the first approach and I would like to know if it's better
to use the second approach. I always like to improve my code.

Thanks for your help.

Best Regards

 

[Marc Gravell]

If the purpose is just to escape a string, then a static method may be
in order; static methods are usually good fits for utility calls.
HOWEVER! You *really* need to look into parameterised SQL. Escaping
*will* eventually let through a duff call. Parameters are *far* more
robust, while retaining compatiblity with most providers.

Marc

 

[Guest]

Thanks for your reply Marc. Yes, good idea, I think a static class will do
the job.
Nevertheless, I'm curious about those two approaches I mentioned before.
Does the second approach is better than the first approach?

Thanks a lot

Best Regards

 

[Marc Gravell]

They both involve creating a meaningless object for no reason (either
everytime, or near-enough every time), and both involve separately
checking a property (for a single value) when a return value would
suffice, so there is little to sing about in either. The pattern
itself is fine... it compares to a number of similar things such as
DbConnectionStringBuilder - the main difference being that such
classes only make sense if you have lots of properties you want to
look at, not just the one. In such patterns, both mutable and
immutable styles are common - by which, I mean e.g. Url accepts a
string into the ctor, and has get (only) properties;
DbConnectionStringBuilder has a string ctor and a default (empty)
ctor, and has both get and set properties. Both patterns are valid;
however. in this case, all you really need is something like:

// note: sql should typically use parameters...
public static EscapeSqlQuery(string query) {
// do some manipulations
return result;
}

and then you can call:

// compare to sqlQueryBD(new sqlCompat("select * from
table").sqlText);
// (I would use "common" names comparable to ExecuteNonQuery etc, but
// the return data isn't clear so I can't extrapolate)
InvokedEscapedSqlQuery(EscapeSqlQuery(query));

As a stylistic recommendation, can I suggest you look at your naming
conventions? None of the names cited are especially meaningful (to me
at least); they might make sense to you now... but in 12 months time?

Marc

 

[Guest]

Thanks a lot for your help Marc.

I really appreciate it.

Best Regards

 

[Guest]

Hi, I'm doing an application using C# and I have this question:

I have a method called sqlQueryBD which receives a string sql query and
executes it against a database.
I also have a class called sqlCompat which has one property "sqlText" with
its getter and setter methods. The purpose of this class to translate the
receiving sql query into an "escaped" sql query, for instance:
insert into people values ('cod001','Carl
Johnson','\\computer\directory\file01.txt')
is translated into:
insert into people values ('cod001','Carl
Johnson','\\\\computer\\directory\\file01.txt')

When you have put together the values into a query, it's too late to
safely escape the values. If someone uses apostrophes somewhere:

insert into people values ('a00','John 'Noone'
Doe','\\computer\directory\file01.txt')

How should the code know that the fourth and fifth apostrophes are to be
escaped?

You need to escape each value before you put it in the query. You should
consider using parameters, as [someone, can't see the thread right
now] suggested.