• Prime Mover (unregistered)

    And don't tell me, the code continues with:

    if (row["Appointment_Num"].ToString() == row["Appointment_Num"].ToString())
{
    bool b1 = String.IsNullOrEmpty(results.messages[1].error_text);
    if (b1 == true)
    {
        row["Message_Id"] = ($"{results.messages[1].message_id}");
        row["message_count"] = ($"{results.message_count[1] }");
        row["message_cost"] = ($"{results.messages[1].message_price }");
        row["error_text"] = ($"{results.messages[1].error_text }");
        row["network"] = ($"{results.messages[1].network }");
        row["to"] = ($"{results.messages[1].to }");
        row["status"] = ($"{results.messages[1].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[1] }");
        row["message_cost"] = ($"{results.messages[1].message_price }");
        row["error_text"] = ($"{results.messages[1].error_text }");
        row["network"] = ($"{results.messages[1].network }");
        row["to"] = ($"{results.messages[1].to }");
        row["status"] = ($"{results.messages[1].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();
    }
}

    ... and so on.

  • NoLand (unregistered)

    However, in terms of a SLOC per task accomplished metric, it's a productive little piece of code.

  • Hans (unregistered)

    I especially love that line:

    row["communicationtype_num"] = ($"1");

    It shows the wizard really didn't understand this incantation

  • okdpm (unregistered)

    bool b1 = String.IsNullOrEmpty(results.messages[0].error_text);

    They didn't want to do the IsNullOrEmpty check twice, so they stuffed it into a boolean

    You're overlooking the main reason for making a variable out of it: the meaningful name.

  • xorium (unregistered)

    And not even a complaint about boolean == true / false?

  • Debra (unregistered)

    This must be auto-generated ... right? RIGHT?!

  • (nodebb)

    People not understanding parentheses is endemic. I see a lot of "return(expression)" code where I work. I occasionally ask the author if they think 'return' is a function call. Also, wonderfully, in certain versions of C++, that parenthesis stops certain optimisations from being used

  • Curmudgeon (unregistered) in reply to okdpm

    Actually, it's a way to avoid a race condition in case results.messages[0].error_text changes between the to "if" statements : )

  • MaxiTB (unregistered) in reply to Hans

    He doesn't understand it at all, because of: row["Sent_Date"] = (String.Format("{0:yyyy-MM-dd hh:mm:ss}", DateTime.Now));

    which is:

    row["Sent_Date"] = $"{DateTime.Now}:yyyy-MM-dd hh:mm:ss";

    Not to mention that it is offical naming convention since 1.0 that you use keywords over types, so it's string.Format() not String.Format().

  • my name is missing (unregistered)

    Clearly if you sacrifice enough code to the gods it will work, no matter how bad.

  • Duston (unregistered) in reply to okdpm

    "the meaningful name." As well as the logging and debugging possibilities. I mean logger.debug((String.IsNullOrEmpty(results.messages[0].error_text)) is so much more complicated than logger.debug(b1);

  • anon (unregistered)

    Because it's easier for the calling code to check for a magic value than it is to check for null.

    Sigh.

  • Dan Bugglin (google)

    The fact they are using row as a dictionary lookup means typos in key names will go undiagnosed until runtime which is undesirable. It's much better to make a class which defines the limited properties for the object and then deserialize your data to that class (for example with Entity Framework or a System.Text.Json or whatever). Then the compiler can check your work on field names automatically.

  • Argle (unregistered)

    When I was teaching, I saw code along these lines. I know it's not fair to compare what a student might do in class to work that's allegedly done by a professional. I'm not going to do that here, but I will say that I got a "deer in the headlights" look when I would simply ask my students "why?" Boy, would I ever like to find this programmer and ask "why" about most of this... if only for the fun of watching the funny faces.

  • So humble (unregistered)

    It looks like the programmer's constantly checking whether something is equal to itself just to make sure the CPU is working in general. Don't take anything for granted!

  • Two Pi Man (unregistered)

    I get it. It's that they didn't write this using ternery if operators, right?

  • Goose (unregistered)

    With regards to the parentheses, he could be working on a system that uses that to denote internalized strings that shouldn't be handled by internationalization. I've seen a few internationalization systems that denote non-translatable strings in that way. That said, its still wrong, because none of those are actual strings that would ever get picked up for externalizing for translation.

  • MaxiTB (unregistered) in reply to Dan Bugglin

    I am pretty sure it's not a Dictionary but a DataTable. So basically this code is over a decade old.

  • Dude (unregistered) in reply to MaxiTB

    The string interpolation used everywhere (except the DateTime formatting, as noted previously) would indicate otherwise. That's only been around for a few years.

  • löchlein deluxe (unregistered) in reply to Curmudgeon

    Yeah, don't remind me of those global auto-erasing errno variables which you can read exactly once.

  • Erwin (unregistered)

    They use two conditionals instead of if/else to prevent a runtime error when b1 equals FILE_NOT_FOUND

  • Chris (unregistered) in reply to MaxiTB

    row["Sent_Date"] = $"{DateTime.Now:yyyy-MM-dd hh:mm:ss}";

    The format goes inside the curly braces.

  • (nodebb) in reply to Erwin

    I was beginning to lose faith. Not one of the first twenty messages mentioned the FileNotFound meme.

  • (nodebb)

    The real WTF is having your post held for moderation because of a link back to this site.

  • (nodebb) in reply to thosrtanner

    That's because pre-ANSI versions of C required the parentheses.

Leave a comment on “A Ritual Approach”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article