"One of our clients was having 'issues' with their mission-critical, flagship web application," writes Mark Doyle, "and of course, it was on me to fix them."
"I knew it was going to be a fun day when I saw this as the entirety of deafult.aspx. The code does absolutely nothing."
public partial class _Default : System.Web.UI.Page { protected void Page_Load(object sender, EventArgs e) { if (Session["Client_Database"] == null) return; string myQuery = "SELECT * FROM Users"; IDataReader myReader = DatabaseFactory.CreateDatabase ((string)Session["Client_Database"]) .ExecuteReader(CommandType.Text, myQuery); DataSet myDataSet = new DataSet(); DatabaseFactory .CreateDatabase((string)Session["Client_database"]) .ExecuteDataSet(CommandType.Text, myQuery); } protected void doSomething(object sender, EventArgs e) { Label1.Text = TextBox1.Text; //Cache myCache = CacheFactory; } }
"After a bit of digging, it was pretty clear that the 'issues' were in the data access class. It was named 'summoner.cs'. I'd rather be playing Elder Scrolls:"
public static IDataReader buildDataReader(string connString, string constName) { string myBuildQuery = definitions.loadQueryString(constName); SqlDataReader myBuildDataReader = null; retrieveDataReader(connString, myBuildQuery, ref myBuildDataReader); return myBuildDataReader; } public static IDataReader buildDataReader(string connString, string constName, string vParam1) { string myBuildQuery = definitions.loadQueryString(constName); myBuildQuery = string.Format(myBuildQuery, vParam1); SqlDataReader myBuildDataReader = null; retrieveDataReader(connString, myBuildQuery, ref myBuildDataReader); return myBuildDataReader; } ... snip ... public static void execNonQuery(string connString, string constName, string vParam1, string vParam2, string vParam3, string vParam4, string vParam5, string vParam6, string vParam7, string vParam8, string vParam9, string vParam10, string vParam11, string vParam12, string vParam13, string vParam14, string vParam15, string vParam16, string vParam17, string vParam18, string vParam19, string vParam20, string vParam21, string vParam22, string vParam23, string vParam24, string vParam25, string vParam26, string vParam27, string vParam28, string vParam29, string vParam30, string vParam31, string vParam32, string vParam33, string vParam34, string vParam35, string vParam36, string vParam37, string vParam38, string vParam39, string vParam40) { string[] myArray = { vParam1, vParam2, vParam3, vParam4, vParam5, vParam6, vParam7, vParam8, vParam9, vParam10, vParam11, vParam12, vParam13, vParam14, vParam15, vParam16, vParam17, vParam18, vParam19, vParam20, vParam21, vParam22, vParam23, vParam24, vParam25, vParam26, vParam27, vParam28, vParam29, vParam30, vParam31, vParam32, vParam33, vParam34, vParam35, vParam36, vParam37, vParam38, vParam39, vParam40 }; string myBuildQuery = definitions.loadQueryString(constName); myBuildQuery = string.Format(myBuildQuery, myArray); //Run the nonQuery statement executeTheNonQuery(connString, myBuildQuery); }
"At least summoner.cs
was flexible. There were 40 overloads of each method, just in case you wanted a different number of parameters. Apparently, the developer never learned about the params
keyword. Looking past that, there's a constName
parameter, which conveniently maps to a private string constant in the definitions.cs
file. And what could that contain?
private const string check_user_exists = "SELECT username FROM users WHERE username = '{0}'"; private const string get_user_record = "SELECT * FROM users WHERE username = '{0}'"; private const string get_user_count = "SELECT COUNT(*) FROM users"; ... and so on (200+ times)...
"Okay... there's just one problem: constName
is a string, and these are all constant variables. Could the definitions.loadQueryString(constName)
method use reflection to find what the value of the constant is?
internal static string loadQueryString(string constName) { string myQuery = ""; switch (constName) { case "check_user_exists": myQuery = check_user_exists; break; case "get_user_record": myQuery = get_user_record; break; case "get_user_count": myQuery = get_user_count; break; // and 200+ more } return myQuery; }
"At least the client got their money's worth in terms of lines of code."