Comment On The SQL String

Fistula!

TODO: laugh at all the posts cursing me for doing this and begging Alex to screen them out.*





* With apologies to kastein, who seems so sincere when he does it.

"// ED: line breaks added for clarity"

Love it. :)

And this is how it should be done: clear, well indented, easy to read. This guy should be given the K.I.S.S award!

I like how its all in alphabetical order.

Two words: String.Format

return("<3".Replace("<", "&lt;"));

For every problem there is a solution that is obvious, simple... and wrong.

Imagine reading this before code before he added the line breaks...

I fail to understand what the parentheses are for...

WTF is with all the +""'s and the pointless parentheses?

Unfortunately I don't consider moving your "Sql escaping and quoting" code into your business object that much of a wtf. Sure, someone doesn't know about how to use methods but still...

the below code is not tested, and wouldn't handle enums, but saves me from solid wall of .ToString().Trim().Replace("'","''")

... or someone could use a parameterized query...

static public string SqlList(params object[] values) {

	StringBuilder sb = new StringBuilder;

	bool first = true;

	for (object value in values) {

		if (!first) sb.Write(", ");

		first = false;

		if (value is string) sb.Write("'"+((string)value).Trim().Replace("'","''")+"'");

		else if (value is decimal || value is int || value is float || value is double) sb.Write(value.ToString());

		else throw new NotSupportedException("Unexpected value type: " + typeof(value));

	}

	return sb.ToString();

}



...



return SqlList(this.Acct,

  this.AcctDist,

  this.BatNbr,

  this.CmmnPct,

  this.CnvFact,

  this.ContractID,

  this.CostType,

  this.CpnyID,

  this.Crtd_DateTime,

  this.Crtd_Prog,

  this.Crtd_User,

  this.CuryExtCost,

  this.CuryId,

  this.CuryMultDiv,

  this.CuryRate,

  this.CuryTaxAmt00,

  this.CuryTaxAmt01,

  this.CuryTaxAmt02,

  this.CuryTaxAmt03,

  this.CuryTranAmt,

  this.CuryTxblAmt00,

  this.CuryTxblAmt01,

  this.CuryTxblAmt02,

  this.CuryTxblAmt03,

  this.CuryUnitPrice,

  this.CustId,

  this.DrCr,

  this.Excpt,

  this.ExtCost,

  this.ExtRefNbr,

  this.FiscYr,

  this.FlatRateLineNbr,

  this.InstallNbr,

  this.InvtId,

  this.JobRate,

  this.JrnlType,

  this.LineId,

  this.LineNbr,

  this.LineRef,

  this.LUpd_DateTime,

  this.LUpd_Prog,

  this.LUpd_User,

  this.MasterDocNbr,

  this.NoteId,

  this.OrdNbr,

  this.PC_Flag,

  this.PC_ID,

  this.PC_Status,

  this.PerEnt,

  this.PerPost,

  this.ProjectID,

  this.Qty,

  this.RecordID,

  this.RefNbr,

  this.Rlsed,

  this.S4Future01,

  this.S4Future02,

  this.S4Future03,

  this.S4Future04,

  this.S4Future05,

  this.S4Future06,

  this.S4Future07,

  this.S4Future08,

  this.S4Future09,

  this.S4Future10,

  this.S4Future11,

  this.S4Future12,

  this.ServiceCallID,

  this.ServiceCallLineNbr,

  this.ServiceDate,

  this.ShipperCpnyID,

  this.ShipperID,

  this.ShipperLineRef,

  this.SiteId,

  this.SlsperId,

  this.SpecificCostID,

  this.Sub,

  this.TaskID,

  this.TaxAmt00,

  this.TaxAmt01,

  this.TaxAmt02,

  this.TaxAmt03,

  this.TaxCalced,

  this.TaxCat,

  this.TaxId00,

  this.TaxId01,

  this.TaxId02,

  this.TaxId03,

  this.TaxIdDflt,

  this.TranAmt,

  this.TranClass,

  this.TranDate,

  this.TranDesc,

  this.TranType,

  this.TxblAmt00,

  this.TxblAmt01,

  this.TxblAmt02,

  this.TxblAmt03,

  this.UnitDesc,

  this.UnitPrice,

  this.User1,

  this.User2,

  this.User3,

  this.User4,

  this.User5,

  this.User6,

  this.User7,

  this.User8,

  this.WhseLoc);

Sorry, I think it's a fake. Why should otherwise all the methods be sorted alphabetically by name???

campkev:

WTF is with all the +""'s and the pointless parentheses?

They're adding in single quotes where things need to be single quoted, and adding in an extra single quote in places where there is already a single quote...In mssql and sybase you escape a single quote by adding...another single quote.

They're also adding commas, which, again, makes sense. Not sure if you realize that in many languages "+" is a string concatenation operator.

The query string that comes out of this would be perfectly cromulent, the WTF is that they're doing it "by hand" instead of building it through a method which would remove all the redundant steps.

Anonymous Coward:

Sorry, I think it's a fake. Why should otherwise all the methods be sorted alphabetically by name???

Because it's simpler that way.

Anonymous Coward:

Sorry, I think it's a fake. Why should otherwise all the methods be sorted alphabetically by name???

Your mistake is in thinking that, just because someone is insane enough to do it this way, that they wouldn't still be trying to make it easier to maintain.

Can you imagine working on this thing if it wasn't in alphabetical order? But put it in alphabetical order, and it is much easier to edit the code. Imagine if they were building an insert query or something, where they'd need to have the field names in the same order in TWO places...It'd be virtually impossible if they weren't in order.

See, this is the perfect example that computers are stupid. If any human had to compile and run this POS of code, he would probable bitch slap you in the face for being such a lousy architect.

"I think it speaks for itself"

And it sure has a lot to say!

Satanicpuppy:

campkev:

WTF is with all the +""'s and the pointless parentheses?

They're adding in single quotes where things need to be single quoted, and adding in an extra single quote in places where there is already a single quote...In mssql and sybase you escape a single quote by adding...another single quote.

They're also adding commas, which, again, makes sense. Not sure if you realize that in many languages "+" is a string concatenation operator.

The query string that comes out of this would be perfectly cromulent, the WTF is that they're doing it "by hand" instead of building it through a method which would remove all the redundant steps.

Reread what I wrote and then try again. I didn't write what is with all the +"'"'s , I wrote what is with all the +""'s. +"" adds absolutely nothing.

This... thing... is the equivalent of writing a math problem like this:
((((5+2)+0)+3+1)+0)
instead of like this:
5+2+3+1

D:

"I think it speaks for itself"

And it sure has a lot to say!

It says "find the guy who wrote me and punch him in the genitals!"

CAPTCHA--facilisis (Earth Goddesses are Easy)

amischiefr:

And this is how it should be done: clear, well indented, easy to read. This guy should be given the K.I.S.S award!

Yup, the K.I.S.S. of death for sure...

The formatting might be easy but this is one of those things that started out life as a 5 line bit of code and turned into the 2000 line behemoth you see before ya. Each subsequent developer just added one more line to, um, keep it simple. How many times will we see code like this before someone realizes that "simple" ain't so simple.

campkev:

Satanicpuppy:

campkev:

WTF is with all the +""'s and the pointless parentheses?

They're adding in single quotes where things need to be single quoted, and adding in an extra single quote in places where there is already a single quote...In mssql and sybase you escape a single quote by adding...another single quote.

They're also adding commas, which, again, makes sense. Not sure if you realize that in many languages "+" is a string concatenation operator.

The query string that comes out of this would be perfectly cromulent, the WTF is that they're doing it "by hand" instead of building it through a method which would remove all the redundant steps.

Reread what I wrote and then try again. I didn't write what is with all the +"'"'s , I wrote what is with all the +""'s. +"" adds absolutely nothing.

This... thing... is the equivalent of writing a math problem like this:
((((5+2)+0)+3+1)+0)
instead of like this:
5+2+3+1

Oh, I see now. Sorry, was trying not to look at it too closely for fear it would infect my brain.

It's copy/paste-itis. The ""s are there because most lines need a "'", and where there wasn't a need for a "'" (because the datum is numeric, for example), they just took out the ' and left "" behind.

I have no explanation about the parenthesis though. That's just fricking wrong.

I saw the parens at the start of the wtf and thought of this:

Satanicpuppy:

In mssql and sybase you escape a single quote by adding...another single quote.

Not sure if you're trying to imply that escaping a single quote with another single quote is a stupid decision by MSSQL and Sybase, but I'm pretty sure it's because that's what the SQL standard says. Postgres does it the same way, and MySQL at least supports it, even though MySQL seems to prefer using a backslash for the escape character.

Wheaties:

How many times will we see code like this before someone realizes that "simple" ain't so simple.

The answer, my friend, is blowin' in the wind, the answer is blowin' in the wind.

I'm not sure what language this is exactly: there is probably a WTF here in that you have a class with 109 attributes to begin with.

There may be reasons for wanting to output a record as a string and some neater patterns to handle the different types might help, but it would still look stupid however you did it just because there are so many.

Dragnslcr:

Satanicpuppy:

In mssql and sybase you escape a single quote by adding...another single quote.

Not sure if you're trying to imply that escaping a single quote with another single quote is a stupid decision by MSSQL and Sybase, but I'm pretty sure it's because that's what the SQL standard says. Postgres does it the same way, and MySQL at least supports it, even though MySQL seems to prefer using a backslash for the escape character.

It's normal for SQL, but it's weird in general. The backslash is much more commonly used in programming languages, and you'd think that the SQL standard would reflect that.

That being said, I don't actually have a preference. I like being able to use backslashes, but the logic for using the single quote as an escape character is easier, and having multiple escape characters, a la MySQL is a bad idea.

Perhaps all the parenthesis and the empty + ""ing is to make sure the system interprets everything as strings, instead of trying to cast everything to numerics and returning field1+field2 as a math equasion. + can be used as both numeric additions and string concats. Maybe that language will always try to cast to numerals when there's no Option Strict (or equivalent) set in the code/compiler.

Hey, isn't that a table from Solomon?

Cbuttius:

I'm not sure what language this is exactly: there is probably a WTF here in that you have a class with 109 attributes to begin with.

It looks to be C#.

If you look at it sideways, it looks like an ASCII drawing of the Houses of Parliament in London. In this sense, at least, it is elegant and well-maintained.

In this case, KISS would stand for: Keep It Stupid, Simple!

"There are almost an infinite number of ways to perform any task; yet, there is only one best way. That is, our chances of guessing the correct way is basically zero. Nature has endowed us with “intuition” which quickly gives us a workable solution – one which is usually better than most solutions. However, among all possible solutions, there is only one best method. Since the intuitive solutions are generally arrived at quickly with insufficient information, they are seldom optimum. Therefore, your chances of hitting the optimum method using intuition are very low." Chang, Fundamentals of Piano Practice

Is your senior architect a closet LISP fan?

Jess:

In this case, KISS would stand for: Keep It Stupid, Simple!

How about "KISaS"?

JohnBigboote:

Cbuttius:

I'm not sure what language this is exactly: there is probably a WTF here in that you have a class with 109 attributes to begin with.

It looks to be C#.

If you look at it sideways, it looks like an ASCII drawing of the Houses of Parliament in London. In this sense, at least, it is elegant and well-maintained.

...and full of useless shit.

wtf!:

"There are almost an infinite number of ways to perform any task; yet, there is only one best way. That is, our chances of guessing the correct way is basically zero. Nature has endowed us with “intuition” which quickly gives us a workable solution – one which is usually better than most solutions. However, among all possible solutions, there is only one best method. Since the intuitive solutions are generally arrived at quickly with insufficient information, they are seldom optimum. Therefore, your chances of hitting the optimum method using intuition are very low." Chang, Fundamentals of Piano Practice

However, a rapidly-conceived intuitive solution which allows the monkey to escape the lion is far more useful than an elegant optimum solution resulting in a well-fed lion.

So what is the upper limit on size for a SQL string? Whatever the limit is, this thing looks like it may be approaching it.

mephisto:

Imagine reading this before code before he added the line breaks...

Thanks. I was really starting to enjoy myself before I read that.

iToad:

So what is the upper limit on size for a SQL string? Whatever the limit is, this thing looks like it may be approaching it.

I think on SQL server it is 2^15 characters... I have once maintained a project where this was surprisingly reached.
<code>SELECT ... FROM ... WHERE CustomerID = 'F9168C5E-CEB2-4faa-B6BF-329BF39FA1E4' OR CustomerID = 'F9168C5E-CEB2-4faa-B6BF-329BF39FA1E5' OR CustomerID = 'F9168C5E-CEB2-4faa-B6BF-329BF39FA1E6' OR CustomerID = 'F9168C5E-CEB2-4faa-B6BF-329BF39FA1E7' OR CustomerID = 'F9168C5E-CEB2-4faa-B6BF-329BF39FA1E8' OR CustomerID = 'F9168C5E-CEB2-4faa-B6BF-329BF39FA1E9' OR
...
</code>

No, no, no, no. Things should be simple. Line-breaks create many lines, it should be left as a simple, single one.

This isn't the SQL being executed, it the result of this string concat that's being executed... which may be larger or smaller than what you see and probably a lot smaller.

It's a shame the code has sql injection

if acct is something like "stuff \' --"
the replace would make that "stuff \'' --" getting you out of the quotes

this.Acct.Replace("'", "''")

"Nature has endowed us with “intuition” which quickly gives us a workable solution – one which is usually better than most solutions." Chang, Fundamentals of Piano Practice

I know this meme has been used ad nauseam, but the only thing that I can say is... what is this I don't even

LART:

It's a shame the code has sql injection

if acct is something like "stuff \' --"
the replace would make that "stuff \'' --" getting you out of the quotes

this.Acct.Replace("'", "''")

nope, quoted sql doesn't have backslash as a predefined escape character. (when you're doing LIKE matching you can specify an escape character with the ESCAPE keyword, see http://sqlserver2000.databases.aspfaq.com/how-do-i-search-for-special-characters-e-g-in-sql-server.html)

If they're using sql server, I wonder if they've noticed they are using the ascii string literals, not the unicode string literals...

Anonymous Coward:

Sorry, I think it's a fake. Why should otherwise all the methods be sorted alphabetically by name???

If I'm calling a bunch of methods one after the other and the order of calls is not relevant, I'll always put the shortest first and the longest last (by "short" and "long" I literally mean the number of characters in the name of the method being called). This isn't alphabetic but it is still an arbitrary order that I impose for seemingly no reason. The point I'm trying to make is that it is not unusual for human beings to create order where none is required. We just kind of do it because order appeals to our logical minds. This is not a sign of fakery and I have no reason to suspect that today's submission is not genuine (I've seen similar code plenty of times). And of course, there is a very real possiblity that this was done deliberately in an attempt to make maintenance easier.

campkev:

with all the +""'s. +"" adds absolutely nothing.

wrong, it adds a lot of garbage to the heap!

;)

SenTree:

wtf!:

"There are almost an infinite number of ways to perform any task; yet, there is only one best way. That is, our chances of guessing the correct way is basically zero. Nature has endowed us with “intuition” which quickly gives us a workable solution – one which is usually better than most solutions. However, among all possible solutions, there is only one best method. Since the intuitive solutions are generally arrived at quickly with insufficient information, they are seldom optimum. Therefore, your chances of hitting the optimum method using intuition are very low." Chang, Fundamentals of Piano Practice

However, a rapidly-conceived intuitive solution which allows the monkey to escape the lion is far more useful than an elegant optimum solution resulting in a well-fed lion.

I am sure the lion would disagree.

When I read this first thing this morning, I let out an audible long and drawn out UUUUUUUUUUUUUUUUUUGH... Everybody was looking at me so I had to act like I messed something up.

grzlbrmft:

SELECT ... FROM ... WHERE CustomerID = 'F9168C5E-CEB2-4faa-B6BF-329BF39FA1E4' OR CustomerID = 'F9168C5E-CEB2-4faa-B6BF-329BF39FA1E5' OR CustomerID = 'F9168C5E-CEB2-4faa-B6BF-329BF39FA1E6' OR CustomerID = 'F9168C5E-CEB2-4faa-B6BF-329BF39FA1E7' OR CustomerID = 'F9168C5E-CEB2-4faa-B6BF-329BF39FA1E8' OR CustomerID = 'F9168C5E-CEB2-4faa-B6BF-329BF39FA1E9' OR
...

Apparently your predecessor hadn't met Mr. Join.

So was this seriously on one line? It's a good idea for TDWTF editors to add line breaks for clarity but I think that whenever this happens you should also include the original code, exactly as it appeared on submission. Sometimes a great WTF is elevated to the next level by nothing more than its excrutiatingly bad composition.

Only if the database system uses a backslash as an escape. Some (Many? Or few? I just use the built-in escape function, so I wouldn't know.) just escape quotes by doubling them, CSV-style.