Representative line



  • I work on reasonably large cog in a huge system of pipes, cogs and magic.  Sometimes, you see total complete WTFs that are worthy of the main page. Sometimes you're just baffled. Sometime, you see this:

        // <snip about 20 lines of comment, justifying why the TYPE and FLAG changing are important>
        if (fieldHasChanged (FieldDef::TYPE) || fieldHasChanged(FldDef::FLAG) || 1)

    Naturally, the justification doesn't include why it is a tautology.

    I checked the history, and this line was witten as-is along with the 20-line comment and the tautology in the first revision. In 2004. 

    I know this isn't the worst WTF in the world, but the actual block the "if" wraps contains a bunch of dubious uses of exceptions, macros and the inner-system anti-pattern. This line just summed it up for me.



  • Are there side effects induced by fieldHasChanged?



  • @Thuktun said:

    Are there side effects induced by fieldHasChanged?

     That doesn't really matter for the if block being executed always, now does it?



  • It does if you want to remove the possibly pointless comments and if statement.



  • what's sad is that fieldHasChanged is actually a macro. the only side-effect of which being that this critical component crashes if you don't call another macro, which we'll call initialiseField() previously. sigh.



  • Has the field changed?

    Are you sure?

    Oh hell, do it anyway!



  • @Weps said:

    That doesn't really matter for the if block being executed always, now does it?
     

    If it throws an error.

     

    You know, from the chapter on brickwalls in the rulebook of "Stopping Your Car" .



  • @Weps said:

    @Thuktun said:

    Are there side effects induced by fieldHasChanged?

     That doesn't really matter for the if block being executed always, now does it?

     

     Sure it does... Although, it's logically equivalent to the following:

     

    // <snip about 20 lines of comment, justifying why the TYPE and FLAG changing are important>
    if (!fieldHasChanged (FieldDef::TYPE)){

    fieldHasChanged(FldDef::FLAG;

    }

    //do whatever was in the if block



  • Looks like someone just added "|| 1" for debugging, rather than change/comment the original condition, and never removed it.



  • @lolwtf said:

    Looks like someone just added "|| 1" for debugging, rather than change/comment the original condition, and never removed it.

    Perhaps, except that the OP had this:
    @pure said:
    I checked the history, and this line was witten as-is along with the 20-line
    comment and the tautology in the first revision.



  • @Scarlet Manuka said:

    @lolwtf said:
    Looks like someone the original developer just added "|| 1" for debugging, rather than change/comment the original condition, and never removed it.
    FTFY


Log in to reply