CodeSOD: A Ritual Approach

Frequent contributor Russell F stumbled across this block, which illustrates an impressive ability to make a wide variety of bad choices. It is written in C#, but one has the sense that the developer didn't really understand C#. Or, honestly, programming.

if (row["Appointment_Num"].ToString() == row["Appointment_Num"].ToString()) { bool b1 = String.IsNullOrEmpty(results.messages[0].error_text); if (b1 == true) { row["Message_Id"] = ($"{results.messages[0].message_id}"); row["message_count"] = ($"{results.message_count[0] }"); row["message_cost"] = ($"{results.messages[0].message_price }"); row["error_text"] = ($"{results.messages[0].error_text }"); row["network"] = ($"{results.messages[0].network }"); row["to"] = ($"{results.messages[0].to }"); row["status"] = ($"{results.messages[0].status }"); row["communicationtype_num"] = ($"1"); row["Sent_Date"] = (String.Format("{0:yyyy-MM-dd hh:mm:ss}", DateTime.Now)); row["Sent_Message"] = row["Sent_Message"].ToString(); } if (b1 == false) { row["Message_Id"] = "0000000000000000"; row["message_count"] = ($"{results.message_count[0] }"); row["message_cost"] = ($"{results.messages[0].message_price }"); row["error_text"] = ($"{results.messages[0].error_text }"); row["network"] = ($"{results.messages[0].network }"); row["to"] = ($"{results.messages[0].to }"); row["status"] = ($"{results.messages[0].status }"); row["communicationtype_num"] = ($"1"); row["Sent_Date"] = (String.Format("{0:yyyy-MM-dd hh:mm:ss}", DateTime.Now)); row["Sent_Message"] = row["Sent_Message"].ToString(); } }

Let's just start on the first line. This entire block is surrounded by a condition: row["Appointment_Num"].ToString() == row["Appointment_Num"].ToString(). If appointment num, as a string, matches appointment num, as a string, we can execute this code.

Inside of that block, we check to see if the error message IsNullOrEmpty. If it is, we'll turn it into a database row. If it's not, we'll also turn it into a database row, but with a hard-coded ID. At first glance, it's weird that they assign the IsNullOrEmpty check to a variable, but when you see that this code is written as two conditionals, instead of an if/else, you realize they forgot that else existed. They didn't want to do the IsNullOrEmpty check twice, so they stuffed it into a boolean.

It's also worth noting that the only difference between the two branches is the Message_Id, so there's a lot of duplicated code in there.

And that duplicated code is its own pile of WTFs. Clearly, the database itself is stringly-typed, as everything gets converted into a string. We mostly avoid using ToString, though, and instead use C#'s string interpolation. Which is'nt really a problem, but most of these fields are already strings. error_text, network, to, status, all strings. And `row["Sent_Message"] is a string, too- and we convert it to a string and store it in the same field.

After all that, I barely have the energy to wonder why they wrapped all of those assignments in parentheses. I suspect they misunderstood C# enough to think it's necessary. This whole thing has the vibe of "programming by incantation and ritual"- if we get the symbols in the right order and express them the right way, it will work, even if we don't understand it.

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!

This post originally appeared on The Daily WTF.

Leave a Reply

Your email address will not be published.