Best Practices turned into Coding Horrors.
-
It is a well known good practice to use
StringBuilders instead of doing manystringconcatenations. Yet, I got really impressed when I saw a document telling to replace things like this:private const string SQL =
"SELECT " +
" ID, " +
" NAME, " +
" BIRTHDAY " +
"FROM " +
" TABLE " +
"WHERE " +
" NAME LIKE @PARAM";By creating the
StringBuildereverytime in the method where the SQL was being used. Maybe I am wrong :doh: , but I really believeconsts aren't doing bad string concatenations all the time.But why would anyone write it like that in the first place??? It's horrible. And let's not even begin to talk about why it should be a stored procedure...
"If you think it's expensive to hire a professional to do the job, wait until you hire an amateur." Red Adair. nils illegitimus carborundum me, me, me
-
But why would anyone write it like that in the first place??? It's horrible. And let's not even begin to talk about why it should be a stored procedure...
"If you think it's expensive to hire a professional to do the job, wait until you hire an amateur." Red Adair. nils illegitimus carborundum me, me, me
Simple... "best practices". :laugh:
-
It is a well known good practice to use
StringBuilders instead of doing manystringconcatenations. Yet, I got really impressed when I saw a document telling to replace things like this:private const string SQL =
"SELECT " +
" ID, " +
" NAME, " +
" BIRTHDAY " +
"FROM " +
" TABLE " +
"WHERE " +
" NAME LIKE @PARAM";By creating the
StringBuildereverytime in the method where the SQL was being used. Maybe I am wrong :doh: , but I really believeconsts aren't doing bad string concatenations all the time.Yeah, this code is incredibly silly. Anybody knows it should be private static readonly string SQL = ;P
-
Yeah, this code is incredibly silly. Anybody knows it should be private static readonly string SQL = ;P
I don't know if you are being serious or if you are joking, after all, you are saying something intermediary... it is not an horror, but it is not right... it's strange.
-
I don't know if you are being serious or if you are joking, after all, you are saying something intermediary... it is not an horror, but it is not right... it's strange.
Edited my post... ;P
-
It is a well known good practice to use
StringBuilders instead of doing manystringconcatenations. Yet, I got really impressed when I saw a document telling to replace things like this:private const string SQL =
"SELECT " +
" ID, " +
" NAME, " +
" BIRTHDAY " +
"FROM " +
" TABLE " +
"WHERE " +
" NAME LIKE @PARAM";By creating the
StringBuildereverytime in the method where the SQL was being used. Maybe I am wrong :doh: , but I really believeconsts aren't doing bad string concatenations all the time.Indeed, not a candidate for StringBuilderhood. And I write it as
private const string SQL =
@"
SELECT ID
, NAME
, BIRTHDAY
FROM TABLE
WHERE NAME LIKE @PARAM
" ;so it prints out nice in error messages [added>>] and I can very easily copy/paste it between a code file and SSMS or other SQL executor.
-
But why would anyone write it like that in the first place??? It's horrible. And let's not even begin to talk about why it should be a stored procedure...
"If you think it's expensive to hire a professional to do the job, wait until you hire an amateur." Red Adair. nils illegitimus carborundum me, me, me
I see little reason to not use a parametrized query like that. OK, if it is static enough top define a const, there is a small case against storing all your query code in the binary instead of the more accessible DB server, but not much else of a case.
-
Indeed, not a candidate for StringBuilderhood. And I write it as
private const string SQL =
@"
SELECT ID
, NAME
, BIRTHDAY
FROM TABLE
WHERE NAME LIKE @PARAM
" ;so it prints out nice in error messages [added>>] and I can very easily copy/paste it between a code file and SSMS or other SQL executor.
:thumbsup:
-
It is a well known good practice to use
StringBuilders instead of doing manystringconcatenations. Yet, I got really impressed when I saw a document telling to replace things like this:private const string SQL =
"SELECT " +
" ID, " +
" NAME, " +
" BIRTHDAY " +
"FROM " +
" TABLE " +
"WHERE " +
" NAME LIKE @PARAM";By creating the
StringBuildereverytime in the method where the SQL was being used. Maybe I am wrong :doh: , but I really believeconsts aren't doing bad string concatenations all the time.The "Best Practice" is not to use string concatenation in a loop. The reason is that under the hood when concatinating two strings, a third string will be created large enough to bold both source strings. The source strings will be copied to that new string, the original string destroyed, then recreated and the contents of the temporary string copied into it, then the temporary string destroyed. The concatination that you are showing should be fine, unless it is being performed in a loop.
-
The "Best Practice" is not to use string concatenation in a loop. The reason is that under the hood when concatinating two strings, a third string will be created large enough to bold both source strings. The source strings will be copied to that new string, the original string destroyed, then recreated and the contents of the temporary string copied into it, then the temporary string destroyed. The concatination that you are showing should be fine, unless it is being performed in a loop.
You are talking about the real Best Practice. But the "Best Practice" is to replace any string concatenation, even in consts, by a StringBuilder.
-
It is a well known good practice to use
StringBuilders instead of doing manystringconcatenations. Yet, I got really impressed when I saw a document telling to replace things like this:private const string SQL =
"SELECT " +
" ID, " +
" NAME, " +
" BIRTHDAY " +
"FROM " +
" TABLE " +
"WHERE " +
" NAME LIKE @PARAM";By creating the
StringBuildereverytime in the method where the SQL was being used. Maybe I am wrong :doh: , but I really believeconsts aren't doing bad string concatenations all the time.Well, best practices are not always the best... :doh:
CEO at: - Rafaga Systems - Para Facturas - Modern Components for the moment...
-
I see little reason to not use a parametrized query like that. OK, if it is static enough top define a const, there is a small case against storing all your query code in the binary instead of the more accessible DB server, but not much else of a case.
It's not the end of the world, certainly; my preference is to keep databasey stuff in the database. It's just neater; besides, all those +++ and line breaks: FUGLY!!!
"If you think it's expensive to hire a professional to do the job, wait until you hire an amateur." Red Adair. nils illegitimus carborundum me, me, me
-
But why would anyone write it like that in the first place??? It's horrible. And let's not even begin to talk about why it should be a stored procedure...
"If you think it's expensive to hire a professional to do the job, wait until you hire an amateur." Red Adair. nils illegitimus carborundum me, me, me
mark merrens wrote:
But why would anyone write it like that in the first place???
How else are you going to code a string which contains SQL? One really long line?
mark merrens wrote:
And let's not even begin to talk about why it should be a stored procedure...
Presumably you mean it should be a proc instead. Perhaps. But some procs might be rather long when expressed as a SQL string - so same problem.
-
Indeed, not a candidate for StringBuilderhood. And I write it as
private const string SQL =
@"
SELECT ID
, NAME
, BIRTHDAY
FROM TABLE
WHERE NAME LIKE @PARAM
" ;so it prints out nice in error messages [added>>] and I can very easily copy/paste it between a code file and SSMS or other SQL executor.
PIEBALDconsult wrote:
And I write it as
So to be clear your code looks like the following? And this format is 'better'?
namespace mystuff.otherStuff
{
//---------------------------------------------------------------------------------------
/// /// This is where I do database stuff
///
//---------------------------------------------------------------------------------------
public static class MyDbConstants
{
private const string SQL1 =
@"
SELECT ID
, NAME
, BIRTHDAY
FROM TABLE
WHERE NAME LIKE @PARAM
" ;private const string SQL2 =
@"
SELECT ID
, NAME
, BIRTHDAY
FROM TABLE_OTHER
WHERE NAME LIKE @PARAM
" ;private const string SQL3 =
@"
SELECT ID
, NAME
, BIRTHDAY
FROM TABLE_OTHER2
WHERE NAME LIKE @PARAM
" ;// 100 other like the above with increasing complexity.
-
The "Best Practice" is not to use string concatenation in a loop. The reason is that under the hood when concatinating two strings, a third string will be created large enough to bold both source strings. The source strings will be copied to that new string, the original string destroyed, then recreated and the contents of the temporary string copied into it, then the temporary string destroyed. The concatination that you are showing should be fine, unless it is being performed in a loop.
KRucker wrote:
The "Best Practice" is not to use string concatenation in a loop.
It still depends on what the "string" is. And it also depends on the impact of the code under use. Most of the time a builder is pointless because it does nothing but obfuscate the code.
-
Well, best practices are not always the best... :doh:
CEO at: - Rafaga Systems - Para Facturas - Modern Components for the moment...
"Best practices" are "at best" someone's opinion. ;) In some cases that opinion may be shared by many, but that doesn't always make it right. After all, at one time, how many people had the opinion the world was flat and the best practice was not to sail too far out? While some things that are considered a best practice I do see reason to use over alternatives, I really don't like the idea of having an arbitrary list of "do these things for best results". They (you know, the "they" that killed Kenny) might as well call it "'boxes to use and not think outside of' practices" instead of "best practices".
-
PIEBALDconsult wrote:
And I write it as
So to be clear your code looks like the following? And this format is 'better'?
namespace mystuff.otherStuff
{
//---------------------------------------------------------------------------------------
/// /// This is where I do database stuff
///
//---------------------------------------------------------------------------------------
public static class MyDbConstants
{
private const string SQL1 =
@"
SELECT ID
, NAME
, BIRTHDAY
FROM TABLE
WHERE NAME LIKE @PARAM
" ;private const string SQL2 =
@"
SELECT ID
, NAME
, BIRTHDAY
FROM TABLE_OTHER
WHERE NAME LIKE @PARAM
" ;private const string SQL3 =
@"
SELECT ID
, NAME
, BIRTHDAY
FROM TABLE_OTHER2
WHERE NAME LIKE @PARAM
" ;// 100 other like the above with increasing complexity.
No, I don't use
consts for SQL. -
Well, best practices are not always the best... :doh:
CEO at: - Rafaga Systems - Para Facturas - Modern Components for the moment...
It's best to avoid "best practices".
-
It's not the end of the world, certainly; my preference is to keep databasey stuff in the database. It's just neater; besides, all those +++ and line breaks: FUGLY!!!
"If you think it's expensive to hire a professional to do the job, wait until you hire an amateur." Red Adair. nils illegitimus carborundum me, me, me
Yes, he could hide the queries in resources files, not out in public code.
-
PIEBALDconsult wrote:
And I write it as
So to be clear your code looks like the following? And this format is 'better'?
namespace mystuff.otherStuff
{
//---------------------------------------------------------------------------------------
/// /// This is where I do database stuff
///
//---------------------------------------------------------------------------------------
public static class MyDbConstants
{
private const string SQL1 =
@"
SELECT ID
, NAME
, BIRTHDAY
FROM TABLE
WHERE NAME LIKE @PARAM
" ;private const string SQL2 =
@"
SELECT ID
, NAME
, BIRTHDAY
FROM TABLE_OTHER
WHERE NAME LIKE @PARAM
" ;private const string SQL3 =
@"
SELECT ID
, NAME
, BIRTHDAY
FROM TABLE_OTHER2
WHERE NAME LIKE @PARAM
" ;// 100 other like the above with increasing complexity.
Looks pretty damned neat to me. I always write out my SQL with each identifier or keyword on its own line. Much easier to read and diagnose.