The Daily WTF: Curious Perversions in Information Technology
Welcome to TDWTF Forums Sign in | Join | Help
in Search

Well, that is one way of doing it...

Last post 11-23-2012 10:22 PM by flabdablet. 4 replies.
Page 1 of 1 (5 items)
Sort Posts: Previous Next
  • 11-23-2012 9:29 AM

    Well, that is one way of doing it...

    Found in production code (not mine, obviously, and slightly anonymised). It works, but...

    unsigned int errmsk=1;
    ...
        while(errmsk > 0)
        {
            if((errstatus & errmsk) ^ (errstatus_old & errmsk))
            {
                switch(errmsk)
                {
                    case 1:
                        sendResponse(address1,(long)(errstatus & errmsk));
                        break;
                    case 2:
                        sendResponse(address2,(long)((errstatus & errmsk)>>1));            
                                            break;
                    case 4:
                        sendResponse(address3,(long)((errstatus & errmsk)>>2));    
                        break;
                    default:
                        break;
                }
            }
            errmsk<<=1;
        }

  • 11-23-2012 10:01 AM In reply to

    • Ben L.
    • Top 10 Contributor
    • Joined on 12-21-2010
    • HELP I'M TRAPPED IN A COMMUNITY SERVER FACTORY
    • Posts 3,431

    Re: Well, that is one way of doing it...

    The responses are being sent to different addresses. There's no WTF here. Except possibly the for-switch loop and the fact that it runs forever if unsigned int has infinite bits.
    mi'e .ben.ly. .i na selju'o mi la lojban
  • 11-23-2012 10:33 AM In reply to

    Re: Well, that is one way of doing it...

    Well, as I said, "it works". But let me count the "whyTF did they do it in this way?"s...

    Using a shifted mask of 1 bit when it all ends up with a huge hardcoded address thing anyway (just use if and &, it's not that many checks...),
    Using an XOR every frigging time instead of just checking if the status has changed in the first line (IIRC the function is only called if the status has changed, even),
    Switching by the (magic) position of the mask,
    Sending "1" to the respective address in the most complicated way I have ever seen, using hardcoded shiftcount
    And using this whole thing to send three(!) possible things.

    Did I forget anything?

    You could code the whole thing a lot easier, better, more readable, and easier to expand. That is the WTF.

  • 11-23-2012 4:55 PM In reply to

    • mihi
    • Top 500 Contributor
    • Joined on 05-10-2008
    • Posts 105

    Re: Well, that is one way of doing it...

    It does not only send 1s, and it does not always send to all addresses. Dunno what it should do, but ATM when errstatus_old is 6 and errstatus is 5, it will send a 1 to address1, a 0 to address2 and nothing (since that bit did not change) to address3. The code may not be the clearest (especially since you can stop after the three bits), and unrolling might make it more readable, but I don't think you can remove the logic to determine whether to send 1 or 0, or the if clause whether a message should be sent. Of course I don't know the protocol, maybe the data or whether something is sent does not matter at all...
  • 11-23-2012 10:22 PM In reply to

    Re: Well, that is one way of doing it...

    mihi:
    Dunno what it should do

    It should do exactly what it does do, except that it should do it like this:

    {
        int changed = errstatus ^ errstatus_old;
        if (changed & 1) sendResponse(address1, (long)!!(errstatus & 1));
        if (changed & 2) sendResponse(address2, (long)!!(errstatus & 2));
        if (changed & 4) sendResponse(address3, (long)!!(errstatus & 4));
    }
    
Page 1 of 1 (5 items)
Powered by Community Server (Non-Commercial Edition), by Telligent Systems