• Kerpal (unregistered)

    Fist

  • that other guy (unregistered)

    Could you not just set up the parameters in the switch and make one call to registerEvent to have a compromise of both approaches?

  • bvs23bkv33 (unregistered)

    project should have three states: open, closed and PROJECT_NOT_FOUND

  • bvs23bkv33 (unregistered)

    project should have three states: open, closed and PROJECT_NOT_FOUND

  • (nodebb)

    Can you even imagine being in these code reviews? I'd rather drink tea with old ladies.

  • (nodebb)

    Nice pitfall: in the first parameter, they call the getStatus() function of the "projectDB" object and in the last, they call it on the "project" object. Just moving this blindly into the switch might break your behavior if not careful (assuming but variables are pointing to different instances)

  • (nodebb)

    Nice pitfall: in the first parameter, they call the getStatus() function of the "projectDB" object and in the last, they call it on the "project" object. Just moving this blindly into the switch might break your behavior if not careful (assuming but variables are pointing to different instances)

  • Little Bobby Tables (unregistered) in reply to Mr. TA

    At my stage of life, I'd rather be drinking tea with old ladies than doing anything to do with software.

  • Wizard (unregistered) in reply to that other guy

    the original is hardly DRY anyway as its calling "project.getDescription()" twice..... How I hate idiots who try to cram as much as they can into "one line" of code. Its not like we're still having to use line numbers and having to re-number them when we want to refactor...

  • G (unregistered) in reply to that other guy

    +1, I'd say the chosen solution wasn't DRY either.

  • Brian (unregistered)

    "A foolish consistency is the hobgoblin of little minds" - Emerson

    Seems like Grenk hasn't quite made the intellectual switch from coder to developer yet; the best he can do is imitate the pattern without understanding when it is (and more importantly, is not) appropriate.

  • Peter (unregistered)

    The original code is an unreadable mess, to be sure. On the other hand, "Grenk" does have a point about DRY. There are other options worth exploring here. Simply moving each "registerEvent" argument to a separate line would do wonders, for instance. As would storing the condition we keep checking.

    bool isClosed = projectDB.getStatus().equals(StatusProjectEnum.CLOSED);

    if (isClosed) { //snip: re-open the project } else { //snip: close the project }

    registerEvent( isClosed ? TypeEventProjectEnum.ENABLED : TypeEventProjectEnum.DISABLED, project.getId(), sessionUser, (isClosed ? "Enabled project " : "Disabled project ") + project.getDescription());

    getDao().update(project);

  • (nodebb)

    I was surprised Grenk was a name, and not "set the language to green and the color to Greek".

  • tbo (unregistered) in reply to bvs23bkv33

    If the internet is good for one thing, it's for taking a joke that was funny years ago and beating it to death until only an uncle could love it.

  • (nodebb) in reply to Peter

    Or, and I think this is what "that other guy" was suggesting: (and let's see if I understand correctly about Markdown being a thing here)

    TypeEventProjectEnum eventType;
string eventDescription;

switch(project.getStatus())
{
    case CLOSED:
    {
        //snip: re-open the project
        eventType = TypeEventProjectEnum.ENABLED;
        eventDescription = "Enabled project " + project.getDescription();
        break;
    }
    case OPEN:
    {
        //snip: close the project
        eventType = TypeEventProjectEnum.DISABLED;
        eventDescription = "Disabled project " + project.getDescription();
        break;
    }
}
registerEvent(eventType, project.getId(), sessionUser, eventDescription);

getDao().update(project);
  • Kurbein (unregistered)

    More readable, and more maintainable... what happens to the ternary when we try to add a third project status, let's say ONHOLD? Either you turn your just-messy-ternary into a OMG-WTF-uber-messy-nested-ternary, or you refactor at the risk of introducing bugs.

    WRT emurphy's proposal... I would argue that for so simple parameter passing, I prefer to have the explicit values of the parameters directly at the point of call. I find it more difficult to read when I have to move my eyes up-and-down the code to find the actual values of the parameters.

    When the values come from a somewhat complex calculation, there is no choice... but when passing simple values, explicitly writing them at call-site is better, even if DRYness is somewhat sacrificed. Of course, just MY opinion...

  • (nodebb)

    With just two possible cases, what’s the point of the switch that an if wouldn’t do just as well?

  • PenguinF (unregistered) in reply to Gurth

    The switch may be for people who have trouble with the law of excluded middle, who think the following code is sensible: if (isConnected == true) { ... } else if (isConnected == false) { ... } else { /* mysterious things happen here */ }

  • sizer99 (google)

    The original code isn't DRY either, because Grenk pissed all over it.

  • Chris (unregistered)

    It may be possible that attempting to close or open the project fails for some reason. However, that can easily be handled by a second switch (or for-else) statement.

  • shcode (unregistered) in reply to bvs23bkv33

    agreed. and a ternary statement should have three parts: true, false and wtf am i doing

  • Dlareg (unregistered) in reply to Gurth

    That you cannot write """ case CLOSED: """

  • I dunno LOL ¯\(°_o)/¯ (unregistered)

    enum ProjectStatus { CLOSED, OPEN, PROJECT_NOT_FOUND };

    A "default: assert(false);" would be a good idea here. Unless you specifically want nothing to happen in an unexpected state, you should always use a safety net.

  • jay (unregistered)

    "How I hate idiots who try to cram as much as they can into "one line" of code."

    Besides, I've worked for companies that measure productivity by lines of code written per unit time. Cramming too much in one line makes it look like your productivity sucks.

  • Steve (unregistered) in reply to jay

    Shops that count lines of code deserve what they get

  • chrtol (unregistered)

    I came here to say what emurphy said. Kurbein, I'd rather see the function call once and have it not take up 3 lines than have the arguments computed on the line where the function's called.

    An advantage of using switches over conditionals here (assuming whatever language being used here behaves normally, this is true with C) is that if you forgot a case defined by the enum type, your compiler will yell at you. "if (condition A) ... else if (condition B)" doesn't complain when condition C is added.

  • Excessive Redundancy (unregistered)

    I am surprised that noone has identified the REAL WTF: why tf is the state of the project stored redundantly twice, once as the event type, and then again in the description?

Leave a comment on “A Switch for Grenk”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article