CodeSOD: String Up Your Replacement

Generating SQL statements is a necessary feature of many applications. String concatenation is the most obvious, and also the most wrong way to do this. Most APIs these days offer a way to construct SQL statements out of higher-level abstractions, whether we're talking about .NET's LINQ, or the QueryBuilder objects in many languages.

But let's say you're doing string concatenation. This means you need to have lots of literals in your code. And literal values, as we know, are bad. So we need to avoid these magic values by storing them in variables.

That, at least, seems to be the logic behind this code. Richard G just inherited this C# code from a senior engineer who left:

String SQLstr2;String cDate = ("'Date'");String c0 = ("'0'");String c1 = ("'1'");String c2 = ("'2'");String c3 = ("'3'");String c4 = ("'4'");String c5 = ("'5'");String c6 = ("'6'");String c7 = ("'7'");String c8 = ("'8'");String c9 = ("'9'");String c10 = ("'10'");String c11 = ("'11'");String c12 = ("'12'");String c13 = ("'13'");String c14 = ("'14'");String c15 = ("'15'");String c16 = ("'16'");String c17 = ("'17'");String c18 = ("'18'");String c19 = ("'19'");String c20 = ("'20'");String c21 = ("'21'");String c22 = ("'22'");String c23 = ("'23'");// more codeDouble[] CancelShiftArray = new Double[24];String sPGA;String sDate;// more codeSQLstr2 = "INSERT INTO TBL_OUTPUT_DAILY (PGA ," + (cDate.Replace("'", "\"")) + "," + (c0.Replace("'", "\"")) + "," +(c1.Replace("'", "\"")) + "," + (c2.Replace("'", "\"")) + "," + (c3.Replace("'", "\"")) + "," + (c4.Replace("'", "\"")) + "," +(c5.Replace("'", "\"")) + "," + (c6.Replace("'", "\"")) + "," + (c7.Replace("'", "\"")) + "," + (c8.Replace("'", "\"")) + "," +(c9.Replace("'", "\"")) + "," + (c10.Replace("'", "\"")) + "," + (c11.Replace("'", "\"")) + "," + (c12.Replace("'", "\"")) + "," +(c13.Replace("'", "\"")) + "," + (c14.Replace("'", "\"")) + "," + (c15.Replace("'", "\"")) + "," + (c16.Replace("'", "\"")) + "," +(c17.Replace("'", "\"")) + "," + (c18.Replace("'", "\"")) + "," + (c19.Replace("'", "\"")) + "," + (c20.Replace("'", "\"")) + "," +(c21.Replace("'", "\"")) + "," + (c22.Replace("'", "\"")) + "," + (c23.Replace("'", "\"")) +") VALUES ('" + sPGA + "' , to_timestamp('" + sDate + "','dd-mm-yyyy') ,'" + CancelShiftArray[0] + "','" + CancelShiftArray[1] + "','" +CancelShiftArray[2] + "','" + CancelShiftArray[3] + "','" + CancelShiftArray[4] + "','" + CancelShiftArray[5] + "','" +CancelShiftArray[6] + "','" + CancelShiftArray[7] + "','" + CancelShiftArray[8] + "','" + CancelShiftArray[9] + "','" +CancelShiftArray[10] + "','" + CancelShiftArray[11] + "','" + CancelShiftArray[12] + "','" + CancelShiftArray[13] + "','" +CancelShiftArray[14] + "','" + CancelShiftArray[15] + "','" + CancelShiftArray[16] + "','" + CancelShiftArray[17] + "','" +CancelShiftArray[18] + "','" + CancelShiftArray[19] + "','" + CancelShiftArray[20] + "','" + CancelShiftArray[21] + "','" +CancelShiftArray[22] + "','" + CancelShiftArray[23] + "')";

Each one of the cN variables has single quotes in the string. When we use them, we escape the single quote into a double quote. Why not just put the double quote in the original? At a guess, they didn't know how. They knew how to use Replace to escape them, because they found that code on Stack Overflow once, but didn't understand how it worked. I think they also didn't understand how for loops worked.

Or really, how a lot of things worked.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!