Best Practices turned into Coding Horrors.
-
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
const
s 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.
-
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
What's wrong with it? I mean, it's simple enough I'd put it on one line, but I don't see anything in principle wrong with putting it on several, and as C# doesn't have multi-line string constants, you have to write it as it is there. Edit: apparently @ strings will let you do multi-line constants. Making code a stored procedure hides it away from the developer and makes it harder to see. Select queries should almost never be in one because it makes you go and look at the database to find out what the code is doing ... or, to put it another way, those queries are part of the business logic and should be in the code. But I have a somewhat old fashioned view of the database as essentially a minimally intelligent data store.
-
You are talking about the real Best Practice. But the "Best Practice" is to replace any string concatenation, even in consts, by a StringBuilder.
-
What's wrong with it? I mean, it's simple enough I'd put it on one line, but I don't see anything in principle wrong with putting it on several, and as C# doesn't have multi-line string constants, you have to write it as it is there. Edit: apparently @ strings will let you do multi-line constants. Making code a stored procedure hides it away from the developer and makes it harder to see. Select queries should almost never be in one because it makes you go and look at the database to find out what the code is doing ... or, to put it another way, those queries are part of the business logic and should be in the code. But I have a somewhat old fashioned view of the database as essentially a minimally intelligent data store.
I like the database to take that strain: it's what it is there for, after all. And I never, ever put SQL in code: I either use a view or 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
-
It is a well known good practice to use
StringBuilder
s instead of doing manystring
concatenations. 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
StringBuilder
everytime in the method where the SQL was being used. Maybe I am wrong :doh: , but I really believeconst
s aren't doing bad string concatenations all the time.Unless you're talking about a really old (.NET 1) version of the compiler, this is translated internally into the following il:
.field private static literal string SQL = string('SELECT ID, NAME, BIRTHDAY FROM TABLE WHERE NAME LIKE @PARAM')
As you can see, there's no concatenation in there at all.
I was brought up to respect my elders. I don't respect many people nowadays.
CodeStash - Online Snippet Management | My blog | MoXAML PowerToys | Mole 2010 - debugging made easier -
Unless you're talking about a really old (.NET 1) version of the compiler, this is translated internally into the following il:
.field private static literal string SQL = string('SELECT ID, NAME, BIRTHDAY FROM TABLE WHERE NAME LIKE @PARAM')
As you can see, there's no concatenation in there at all.
I was brought up to respect my elders. I don't respect many people nowadays.
CodeStash - Online Snippet Management | My blog | MoXAML PowerToys | Mole 2010 - debugging made easierI know there is no concatenation. Any const must be resolved at compile time. When I said that I "think it optimizes" I put the :doh: because it is obvious.
-
Unless you're talking about a really old (.NET 1) version of the compiler, this is translated internally into the following il:
.field private static literal string SQL = string('SELECT ID, NAME, BIRTHDAY FROM TABLE WHERE NAME LIKE @PARAM')
As you can see, there's no concatenation in there at all.
I was brought up to respect my elders. I don't respect many people nowadays.
CodeStash - Online Snippet Management | My blog | MoXAML PowerToys | Mole 2010 - debugging made easier -
I know there is no concatenation. Any const must be resolved at compile time. When I said that I "think it optimizes" I put the :doh: because it is obvious.
That was meant for you to beat them round the head with, rather than for your benefit.
I was brought up to respect my elders. I don't respect many people nowadays.
CodeStash - Online Snippet Management | My blog | MoXAML PowerToys | Mole 2010 - debugging made easier -
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.
-
No, I don't use
const
s for SQL. -
"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".
Chad3F wrote:
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?
At the time when people did in fact think the world was flat then in fact is was foolhardy to sail too far out because one was very likely to not return. Which was a loss both in lives and commerce.
-
PIEBALDconsult wrote:
No, I don't use
const
s for SQL.Then we have a term definition problem because I responded to what you said... "And I [PIEBALDconsult] write it as ...private const string SQL"
I meant "that".
-
Chad3F wrote:
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?
At the time when people did in fact think the world was flat then in fact is was foolhardy to sail too far out because one was very likely to not return. Which was a loss both in lives and commerce.
Yes, those that sailed too far likely had a greater chance of something going wrong (per voyage).. but statistically over all, I expect out of all ships/lives lost that only a small percentage were from those sailing out too far. Most had no reason to try (unless they were explorers), so most of the risk was while in known waters (so why not have a best practice of never sailing period). This would be like most people not routinely driving more than 1000 miles (or kilometers) from their home (unless it was required of their job).. hence the [paraphrased] expression of "most [driving] accidents occur within X distance of ones home", simply because that is where the are [on the road] most of the time. In the end, just because something is done for the wrong reasons (i.e. to not fall off the end of the world) and happens to be useful (i.e. less lost lives/cargo), doesn't justify the basis (a variation of "the ends don't justify the means" I suppose). As an example, if a parent tells a young child not to talk to strangers, "just because", but not why explain why (since the child would unlikely be capable of comprehending why anyway), then that may work on the short term.. but, if left at that, in the long term it could be disastrous. Imagine if that child follows that directive to the letter "just because" their parent said so and one day there is a house fire, firefighters come and surround the house with the kid still inside. Now the child sees all these strangers and has to make a choice.. does he go outside, where all these strangers are, and break the rule.. or does he stay inside (or even hide, since several strangers are entering the house for some reason) to avoid them? A slightly contrived scenario (but not impossible) that illustrates how blindly following a "rule" without understanding why can [eventually] lead to a worse outcome than what the rule was indented to avoid. In some cases, like a young child's limited comprehension, you have to try and account for exceptions when stating the rule.. but when comprehension is possible, ignorant, "just because" logic is never an acceptable reason to do something. One should know _why_ something is better to use than another, in which case they shouldn't _need_ to be told to what [specifically] to do for the best, as it is the automatic choice (including _when_ to break the default choice). If I know a wood chipper chops up materials you put in it, and I know one is [potentially] running, then I implicitly choose not to put my hand in it.. I don't need to be told
-
What's wrong with it? I mean, it's simple enough I'd put it on one line, but I don't see anything in principle wrong with putting it on several, and as C# doesn't have multi-line string constants, you have to write it as it is there. Edit: apparently @ strings will let you do multi-line constants. Making code a stored procedure hides it away from the developer and makes it harder to see. Select queries should almost never be in one because it makes you go and look at the database to find out what the code is doing ... or, to put it another way, those queries are part of the business logic and should be in the code. But I have a somewhat old fashioned view of the database as essentially a minimally intelligent data store.
-
I meant "that".
-
Yes, those that sailed too far likely had a greater chance of something going wrong (per voyage).. but statistically over all, I expect out of all ships/lives lost that only a small percentage were from those sailing out too far. Most had no reason to try (unless they were explorers), so most of the risk was while in known waters (so why not have a best practice of never sailing period). This would be like most people not routinely driving more than 1000 miles (or kilometers) from their home (unless it was required of their job).. hence the [paraphrased] expression of "most [driving] accidents occur within X distance of ones home", simply because that is where the are [on the road] most of the time. In the end, just because something is done for the wrong reasons (i.e. to not fall off the end of the world) and happens to be useful (i.e. less lost lives/cargo), doesn't justify the basis (a variation of "the ends don't justify the means" I suppose). As an example, if a parent tells a young child not to talk to strangers, "just because", but not why explain why (since the child would unlikely be capable of comprehending why anyway), then that may work on the short term.. but, if left at that, in the long term it could be disastrous. Imagine if that child follows that directive to the letter "just because" their parent said so and one day there is a house fire, firefighters come and surround the house with the kid still inside. Now the child sees all these strangers and has to make a choice.. does he go outside, where all these strangers are, and break the rule.. or does he stay inside (or even hide, since several strangers are entering the house for some reason) to avoid them? A slightly contrived scenario (but not impossible) that illustrates how blindly following a "rule" without understanding why can [eventually] lead to a worse outcome than what the rule was indented to avoid. In some cases, like a young child's limited comprehension, you have to try and account for exceptions when stating the rule.. but when comprehension is possible, ignorant, "just because" logic is never an acceptable reason to do something. One should know _why_ something is better to use than another, in which case they shouldn't _need_ to be told to what [specifically] to do for the best, as it is the automatic choice (including _when_ to break the default choice). If I know a wood chipper chops up materials you put in it, and I know one is [potentially] running, then I implicitly choose not to put my hand in it.. I don't need to be told
Chad3F wrote:
and happens to be useful (i.e. less lost lives/cargo), doesn't justify the basis
Yes as a matter of a fact it does. Most software development exists to fulfill a need, often either an explicitly or even implicitly commercial. Just as sailing a ship did. Exploring might be 'fun' or 'cool' but the rate of return is very low. Whereas staying with the known routes provided a known rate of return. And that is what businesses want. They don't want fun/cool. They want money.
Chad3F wrote:
Teach that same person 10 concepts, with understanding, and now they know 100's or even 1000's of things (i.e. the meaningful combinations of those 10 concepts)
Of course that is what one wants. But one also wants every one to be a multi-talented genius as well and it just doesn't happen.
-
I like the database to take that strain: it's what it is there for, after all. And I never, ever put SQL in code: I either use a view or 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
Oh yes, I have to agree. Prevents SQL injection attacks.