• Hans (unregistered)

    Yes it probably is generated code. Generated by Excel, of course ...

  • The Dave G (unregistered)

    Truly a WTF of epic proportions. I shudder to see what follows.

  • Naomi (unregistered)

    I guess he never learned about loops or arrays. I guess he never asked questions, because he should know this already.

  • (nodebb)

    If he didn't figure it out when he typed the second nearly identical line, he will not figure it out after 10, 100, 1000, 1900, or any number of lines.

  • Chronomium (unregistered) in reply to Mr. TA

    I give a pass for a second identical line. If there's only two of something, that's probably not worth turning into an array.

    Maybe a third identical line is okay. It depends on the situation.

    Once you get to a fourth, that's when you should reconsider.

  • Duston (unregistered)

    I can hear him now.. 'No loop for you!"

  • Sole Purpose Of Visit (unregistered) in reply to Chronomium

    Law Of Three, basically.

    There are times when this is hard to apply. This is not one of them.

  • charles (unregistered)

    Um. Maybe he generated the file with another program?

  • (nodebb) in reply to charles
    Um. Maybe he generated the file with another program?

    And that makes it better?

  • (nodebb)

    Even though the dummy in question doesn't like loops, I bet he's a huge fan of the Hyperloop.

  • (nodebb) in reply to Chronomium

    Nah. If the same pattern repeats, or there's a possibility it will roast, any number of times other than exactly one, put it in a loop right away. This includes zero repetitions, too.

  • (nodebb) in reply to Mr. TA

    Repeat not roast

  • DQ (unregistered) in reply to Mr. TA

    Repeat not roast

    Repeat not roast

    Repeat not roast

    Repeat not roast

    Damn, should have put it in a loop

  • (nodebb)

    Using a loop would give us this:

    for i=1; i<=1900; i++ { String q1 = form.getQ1()!=null?request.getParameter("question_" + form.getQ1().getId()):null; String q2 = form.getQ2()!=null?request.getParameter("question_" + form.getQ2().getId()):null; String q3 = form.getQ3()!=null?request.getParameter("question_" + form.getQ3().getId()):null; <snip> String q1899 = form.getQ1899()!=null?request.getParameter("question_" + form.getQ1899().getId()):null; String q1900 = form.getQ1900()!=null?request.getParameter("question_" + form.getQ1900().getId()):null; }

    Addendum 2021-07-15 10:20: EDIT: I didn't type it this way, but if it were all on one line that would be even better!

  • Happy Coder (unregistered)

    Well this is some loop unrolling! Never thought one could take optimization that far.

  • Carl Witthoft (google)

    But what about form.getQ9&3/4() ?

  • Shill (unregistered)

    Are you sure there isn't some special handling for question 837?

  • (nodebb)

    While i can see that is aching for a loop, how do you do the getQn() appropriately?

    And I'd sort of like to know what the qn variables are used for at the end. Or maybe the function just returns without using them...

  • The Shadow Knows (unregistered)

    Doesn't matter how the code was created, having a form with 1900 questions seems a bit excessive.

  • Anonymous Coward (unregistered)

    I've worked on a job for 15 years now, and while I am happy to answer the questions of the newer people on my team, there's one gent who has been here as long as me. His knowledge of the system outside his little area where he codes is superficial at best. In 15 years, he should have at least ventured out onto our integration floor and given the system a try every once in a while. I don't answer that guy's questions anymore.

  • In other words (unregistered) in reply to Anonymous Coward

    String q15yrCoworker = form.getQ15yrCoworker()!=null?null:null;

  • pif (unregistered)

    What a noob! That code hurts my eyes. An expert programmer would have given proper names to his variables and methods: q0001 and GetQ0001. No loop, I can condone, maybe he was payed by the line. But those steps at 10, 100 and 1000 are an eyesore!

  • some guy (unregistered)

    Out of boredum, I highlighted the code portion. There is stray whitespace at the end of one line. So many questions: 1. What happened there? 2. Does this rule out code generation? 3. Why didn't their source control catch ... nevermind, I figured that one out.

  • ZZartin (unregistered)

    And I guarantee at least 1 of those is misnumberred.

  • Paulina (unregistered)

    Or lead developer finally decided he wants to use unit tests. So he's now tasking the junior developers to write unit tests for his code. 😣

  • DaveD (unregistered)

    thosrtanner, I think getMethod() might be the way to do that.

    Is this even in a function? The qn variables may be globals.

  • Chris (unregistered)

    Ah, I see the problem. It should be:

    for i = 1..1900
    switch (i) 
        case 1: 
            String q1 = form.getQ1()!=null?request.getParameter("question_" + form.getQ1().getId()):null;
            break;
        case 2:
            String q2 = form.getQ2()!=null?request.getParameter("question_" + form.getQ2().getId()):null;
            break;
        ... etc.
  • wideguy (unregistered)

    The code certainly looks as if it was generated by a tool.

  • (nodebb) in reply to Shill

    Are you sure there isn't some special handling for question 837?

    In that case: Loop with switch-case of if/else. Then at least it is clear at a glance that there is special handling.

  • (nodebb) in reply to wideguy

    The code certainly looks as if it was generated by a tool.

    I can't decide if the double-meaning of your reply is intended or not.

  • MaxiTB (unregistered)

    Yeah, as a C# I am never surprised to see bad Java code (because it's often the first language people pick up today and code quality usually gets better with experience and not the other way around), so I make some guesses:

    I think the issue is with whatever form is; it seems that every form input element is an object with a concrete name while in this case getQuestion(int index) would make more sense. But honestly, this could be worked around with reflection by invoking the concrete method by name "getQ"+index. Usually I would never recommend reflection to fix bad design decisions (just throw away the code and make it write clean), but we are talking hundreds of lines of code here... Jeez.

  • (nodebb) in reply to MaxiTB

    (just throw away the code and make it write clean)

    "We don't have automated tests, we don't have access to an environment where we can perform a full test manually, and that one major customer will complain to our manager if anything looks different than before."

    I would like to rewrite so much of our code...

  • Prime Mover (unregistered) 
    for (i = 0; i < ONE_HUNDRED_TIMES; i++) {
    printf ("I will never repeat code that I can put in a loop again.\n");
}
  • iggy (unregistered) in reply to Prime Mover

    You forgot something to make your look work: #define ONE_HUNDRED_TIMES 0

    you are welcome

  • DaveD (unregistered)

    Reminds me of Columbo: "Just one more question"

  • Mikie (unregistered)

    I once wrote something like this with Oracle PL/SQL. I was building a package to to execute dynamic queries using bind variables from an array and had to account for the number of variables in the string. PL/SQL has no way directly use an array of bind variables. An example of this:

    if query_parameters.count=2 then execute immediate v_sql using parameters(1),parameters(2); else if query_parameters.count=3 then execute immediate v_sql using parameters(1),parameters(2),parameters(3); end if;

    So I wrote a script to generate this, and in the script added comments to the generated code: "Do not edit code yourself. You WILL screw it up. Instead use the script at xxxxxx to generate it".

    That was 20 years ago. If I was doing this again, I would have written raw DBMS_SQL calls to parse and bind the query myself.

  • (nodebb) in reply to Prime Mover

    Ah yes, the good old magical numbers rule...

    Let's rewrite the rule as "if you create a constant, the name of which contains the name of a number, you must prefix the name with "MAGICAL_NUMBER".

    for(int i = MAGICAL_NUMBER_ZERO; i < MAGICAL_NUMBER_TEN; i += MAGICAL_NUMBER_ONE) {
    // ...
}

    Or let's not. I'm pretty sure there will be people, who don't get the hint, and actually create those names.

    Addendum 2021-07-19 05:29: Just to avoid offense: I assume ONE_HUNDRED_TIMES was named ironically.

  • jay (unregistered)

    A company I once worked for announced that they were going to start rating programmers by lines of code produced per day. Some of the other programmers were grumbling about this. I said, Hooray! I can easily boost my lines of code per day. Instead of writing a loop, just repeat the code n times. If the bounds of the loop are variable, enclose each occurrence in "if n>=5 ... if n>=6 ..." etc. And don't write something foolish like "x=x+4". Write "x=x+1" four times.

    The company ended up dropping the idea. I was so disappointed.

Leave a comment on “Just a Few Questions”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article