• Keith (unregistered)

    This is so beyond retarded.

  • chep (unregistered)

    Horror! Horror! [:|]<?xml:namespace prefix = o ns = "urn:schemas-microsoft-com:office:office" /><o:p></o:p>

    Unless this person getting paid by the number of lines (s)he produce...  [;)]<o:p></o:p>

     

  • (cs)

    I have met programmers who didn't know how to use bitwise operators, but had coded in C# for a while. That in itself is kinda scary, but I would hope that going to these absurd lengths to inspect file attributes would usually provoke a thought along the lines of, "you know, there's probably a better way of doing this if I just use Google or MSDN...".

  • (cs)

    Luckily, like those extra parts left over from my reassembling my car's engine, bitwise operators are completely optional!

  • (cs)

    Fantastic! This is the kind of person that has business cards from recent jobs with "HTML Programmer" as job title.

    How about doing it the right way and profile both for comparison? It's so stupid the compiler may even figure it out by itself, though.

  • (cs) in reply to kwu

    please please please please please please PLEASE tell me this person does not have a technical degree...  If this person only has a GED that would be forgivable.

    And sorry to burst your bubble, but not even the schnazziest whizbang compiler would turn this gem into bitwise comparisons :*(

  • Derick Bailey (unregistered)

    I'd be especially pissed off if I were that person... i mean, they must have completely left out the System.IO namespace, or at least the System.IO.FileInfo object...

    but then again, who wants to use a built in object in the framework, when you can write such elegant code as that.

  • (cs) in reply to Charles Nadolski

    [Sarcasm and overengineering tags on!]

    Heh, how about this code to replace the first method:

    public bool IsFileReadOnly(int attributes)

    {
        for(int n = sizeof(int)-1; n > 0; n++)
        {
            if(attributes > pow(2, n));
                attribues -= pow(2, n);
        }
        return (bool)attributes;
    }

    [End sarcasm and overengineering]

    EVEN IF you didn't know how to use bitwise operators there's still a better way to do it!  Extra-super-tasty-hotncrispy-bogolicious WTF!

  • crlf (unregistered)

    LMAO. Not only is the method used stupid, but it's wrong as well:

      if (attributes > 63)
      {
        attributes = attributes - 63;
      }

      if (attributes > 32)
      {
        attributes = attributes - 32;
      }

    Should be:

      if (attributes > 63)
      {
        attributes = attributes - 64;
      }

      if (attributes > 31)
      {
        attributes = attributes - 32;
      }
  • diaphanein (unregistered) in reply to Derick Bailey

    I was forced to do something similar to this in a DB2 SQL stored proc.  They *did* forget to include bitwise operators.  I was reduced to using modulus and division operations to check bits.  Was not as ugly as this, and I made sure the comments dictated what and why I was doing it.

  • (cs) in reply to Charles Nadolski
    Charles Nadolski:
    [Sarcasm and overengineering tags on!]

    public bool IsFileReadOnly(int attributes)
    {
        for(int n = sizeof(int)-1; n > 0; n++)
        {
            if(attributes > pow(2, n));
                attribues -= pow(2, n);
        }
        return (bool)attributes;
    }


    To over-engineer my code even further (for the sake of futility):


    public bool IsFileReadOnly(int attributes)

    {

        for(int n = sizeof(int)-1; n > 0; n++)

        {

            if(attributes >= 2 << n);

                attribues -= 2 << n;

        }

        return (bool)attributes;

    }


    The circle is now complete.

  • (cs) in reply to Charles Nadolski

    meh, change that for loop for(int n = sizeof(int)-2; n >= 0; n++).  OBOB!

  • (cs) in reply to Charles Nadolski

    rut roh, and change that from n++ to n--.  Could we please be allowed to edit our posts?

  • (cs) in reply to Charles Nadolski

    Good one!  We can prescribe it to the "I've never heard of bitwise comparison, but I use bitshifts every other line" crowd.  [:P]

  • (cs) in reply to crlf
    Anonymous:

    LMAO. Not only is the method used stupid, but it's wrong as well: 
      if (attributes > 63)
      {
        attributes = attributes - 63;
      }

      if (attributes > 32)
      {
        attributes = attributes - 32;
      }
    Should be: 
      if (attributes > 63)
      {
        attributes = attributes - 64;
      }

      if (attributes > 31)
      {
        attributes = attributes - 32;
      }

    It would still work though, but for the wrong reason.  Suppose the value of attributes is 32, it would fail on if>32, but it would pass on each of the others until it gets to if==1.  The value would be 2 at that point, so it would evaluate to false.  If he did it your way (the "less wrong" way), it would pass on if>31, and fail on everything after that.

  • (cs) in reply to mugs

    I can't tell you how many people I graduated with who didn't really understand bitwise operators and bitshifting... it's not exactly a critical concept that you can't program without.  Someone with an MIS degree I wouldn't expect to have even heard of bitwise operators.

  • (cs) in reply to mugs
    mugs:
    I can't tell you how many people I graduated with who didn't really understand bitwise operators and bitshifting... it's not exactly a critical concept that you can't program without.  Someone with an MIS degree I wouldn't expect to have even heard of bitwise operators.


    "Management Information Systems"?  Are those the people that don't know how to code but make more money than I do? :-P
  • crlf (unregistered) in reply to Charles Nadolski
    Charles Nadolski:
    Charles Nadolski:
    [Sarcasm and overengineering tags on!]

    public bool IsFileReadOnly(int attributes)
    {
        for(int n = sizeof(int)-1; n > 0; n++)
        {
            if(attributes > pow(2, n));
                attribues -= pow(2, n);
        }
        return (bool)attributes;
    }


    To over-engineer my code even further (for the sake of futility):


    public bool IsFileReadOnly(int attributes)

    {

        for(int n = sizeof(int)-1; n > 0; n++)

        {

            if(attributes >= 2 << n);

                attribues -= 2 << n;

        }

        return (bool)attributes;

    }


    The circle is now complete.


    Wow. You guys are both way off:



    for (int n = sizeof(int)*8-1; n > 0; n--)
    {
        if (attributes >= 1 << n) /* No extra semi-colon! */
           attributes -= 1 << n;
    }
  • crlf (unregistered) in reply to mugs
    mugs:
    Anonymous:

    LMAO. Not only is the method used stupid, but it's wrong as well: 
      if (attributes > 63)
      {
        attributes = attributes - 63;
      }

      if (attributes > 32)
      {
        attributes = attributes - 32;
      }
    Should be: 
      if (attributes > 63)
      {
        attributes = attributes - 64;
      }

      if (attributes > 31)
      {
        attributes = attributes - 32;
      }

    It would still work though, but for the wrong reason.  Suppose the value of attributes is 32, it would fail on if>32, but it would pass on each of the others until it gets to if==1.  The value would be 2 at that point, so it would evaluate to false.  If he did it your way (the "less wrong" way), it would pass on if>31, and fail on everything after that.



    Consider the case where attributes is 65  (bit 6 and 0 set).  The first if statement would subtract 63..
  • Unregistered Guy (unregistered) in reply to mugs
    mugs:
    Anonymous:

    LMAO. Not only is the method used stupid, but it's wrong as well: 
      if (attributes > 63)
      {
        attributes = attributes - 63;
      }

      if (attributes > 32)
      {
        attributes = attributes - 32;
      }
    Should be: 
      if (attributes > 63)
      {
        attributes = attributes - 64;
      }

      if (attributes > 31)
      {
        attributes = attributes - 32;
      }

    It would still work though, but for the wrong reason.  Suppose the value of attributes is 32, it would fail on if>32, but it would pass on each of the others until it gets to if==1.  The value would be 2 at that point, so it would evaluate to false.  If he did it your way (the "less wrong" way), it would pass on if>31, and fail on everything after that.

    Yes, the original version does work correctly for 32, but it's broken for 64 (unless, for some reason, 64 is supposed to return true).

  • (cs) in reply to crlf

    Anonymous:


    Consider the case where attributes is 65  (bit 6 and 0 set).  The first if statement would subtract 63..

    Err, yeah, that's the one that wouldn't work. :)  I have no idea why he did that one that way (2^n - 1) and the rest a different way (2^n)

  • (cs) in reply to crlf

    Anonymous:

    Consider the case where attributes is 65  (bit 6 and 0 set).  The first if statement would subtract 63..

    Err, yeah, that's the one that wouldn't work. :)  I'm not sure why he did that one that way (2^n - 1) and the rest a different way (2^n).

  • (cs) in reply to crlf
    Anonymous:
    Wow. You guys are both way off:

    for (int n = sizeof(int)*8-1; n > 0; n--)
    {
        if (attributes >= 1 << n) /* No extra semi-colon! */
           attributes -= 1 << n;
    }


    Heh, we were actually the same person.  And thanks for cleaning up my code!  And no, I didn't even bother to check if it really worked.
  • (cs)

    I just love the title!!!

  • Mr GED (unregistered) in reply to Charles Nadolski

    Are you trying to say people with GED's and no degree's are stupid? I hate to break it to you buddy, but I got a GED and a 9th grade education and I can code well in oh about 16 languages..... not to mention all the stuff I know the college teachers I've met don't or people with a degree for that matter.......

  • Brad Heintz (unregistered)

    I'm really surprised that nobody has coughed up the obvious replacement for the first function:

    public bool IsFileReadOnly(int attributes)
    {
      bool retVal = false;

      if ((attributes % 2) == 1)
      {
        retVal = true;
      }

      return retVal;
    }

    There's no (conceptual) bitwiseness to the modulo operator, right?

  • (cs) in reply to Brad Heintz
    Anonymous:
    There's no (conceptual) bitwiseness to the modulo operator, right?


    Yeah, but an implementational one. %2 will be optimized to &1, and we can't have that, now can we?
  • chep (unregistered) in reply to Charles Nadolski

    Charles Nadolski:
    meh, change that for loop for(int n = sizeof(int)-2; n >= 0; n++).  OBOB!

    n--

  • Uh-nonymous (unregistered) in reply to CornedBee

     Anonymous wrote:
    There's no (conceptual) bitwiseness to the modulo operator, right?


    Yeah, but an implementational one. %2 will be optimized to &1, and we can't have that, now can we?


    Everyone knows (or should know) the most machine-efficient way to test a single bit it testing the sign bit: it's a single assembler instruction (lz or gz). So to test a different bit, just left shift your argument intil the desired bit is in the sign position, then test the signedness of the argument. Like so:

    public bool IsFileReadOnly(int attributes) {
      for(int i=0; i<sizeof (attributes)*8-1="" i="" {="" attributes=""><<=1;
      }
      return (attributes<0)?1:0;
    }
    </sizeof>
  • Diego Mijelshon (unregistered)

    By the way, there is a FileAttributes enum in .NET... so, the above could be:

    <font size="2">return (</font>    <font size="2">(FileAttributes)</font><font size="2">attributes & FileAttributes.ReadOnly) ==</font><font size="2"> FileAttributes</font><font size="2">.ReadOnly;</font>

    Which is not only easier, but also cleaner... (the cast is only necessary if he keeps the attributes as an int instead of a FileAttributs (which is actually also an int))

  • Duh-nonymous (unregistered) in reply to CornedBee
    CornedBee:
    Anonymous:
    There's no (conceptual) bitwiseness to the modulo operator, right?


    Yeah, but an implementational one. %2 will be optimized to &1, and we can't have that, now can we?


    Feh, messed that up...

    Everyone knows (or should know) the most machine-efficient way to test a single bit it testing the sign bit: it's a single assembler instruction (lz or gz).  So to test a different bit, just left shift your argument intil the desired bit is in the sign position, then test the signedness of the argument.   Like so:

    <sizeof (attributes)*8-1="" i="" {=""></sizeof><sizeof (attributes)*8-1="" i="" {=""></sizeof>public bool IsFileReadOnly(int attributes) {
      for(int i=0; i<sizeof(attributes)*8-1; i++) {
          attributes<<=1;
      }
      return (attributes<0)?1:0;
    }

  • (cs) in reply to Mr GED
    Anonymous:
    Are you trying to say people with GED's and no degree's are stupid? I hate to break it to you buddy, but I got a GED and a 9th grade education and I can code well in oh about 16 languages..... not to mention all the stuff I know the college teachers I've met don't or people with a degree for that matter.......


    Dear Mr Sensitive:

    Carefully re-read my post.  I did not write that people with a GED are stupid.  To paraphrase myself, I would say it is understandable if somebody did not know of bitwise operators if they had not experienced a rigorous educational experience.  However, if we look at your post, your knee-jerk reaction betrays an insecurity about your educational level, despite the fact that you are a successful programmer.

    I think we can agree that on the average, person A who has just graduated from highschool will not have the foggiest notion what a bitwise operator is, but a person B with a Computer Science degree better damn well know what a bitwise operator is.

    Thanks for playing!
  • (cs) in reply to Duh-nonymous
    Anonymous:

    Everyone knows (or should know) the most machine-efficient way to test a single bit it testing the sign bit: it's a single assembler instruction (lz or gz).  So to test a different bit, just left shift your argument intil the desired bit is in the sign position, then test the signedness of the argument.   Like so:

    <sizeof (attributes)*8-1="" i="" {=""></sizeof><sizeof (attributes)*8-1="" i="" {=""></sizeof>public bool IsFileReadOnly(int attributes) {
      for(int i=0; i<sizeof (attributes)*8-1="" i="" {="">
          attributes<<=1;
      }
      return (attributes<0)?1:0;
    }

    </sizeof>


    I think you win :) *golf claps*  Good show!
  • (cs) in reply to crlf
    Anonymous:

    LMAO. Not only is the method used stupid, but it's wrong as well: 
      if (attributes > 63)
      {
        attributes = attributes - 63;
      }

      if (attributes > 32)
      {
        attributes = attributes - 32;
      }
    Should be: 
      if (attributes > 63)
      {
        attributes = attributes - 64;
      }

      if (attributes > 31)
      {
        attributes = attributes - 32;
      }

    Bzzt, wrong.  It was a reading-bad-code test.  You know what you got?  F+  (no offense, I just like quoting Tenacious D [:D])

    If "Read Only" is in the 64 position (01000000), and you're going to check for attributes == 1 at the end, the code is right.  What's weird is that  they (a) didn't continue the theme, and (b) don't ever check for something in the 128 position.

    To keep going with this theme, the other checks should have had (-64, -31, -16, -8, -4, -2), (-64, -32, -15, -8, -4, -2), etc.  That would give you a nice long ugly way to tell what the bit at a certain position was set to, because you'd get back either a 1 or a 0.  Of course, all of these checks would fail if attributes was ever > 128.

  • Boojum (unregistered)

    Bah!  As long as we're being silly about it, why not?

    public bool IsFileReadOnly(int attributes) {
    while ( attributes > 1 )
        attributes -= 2;
    return attributes > 0;
}
  • DT (unregistered) in reply to Brad Heintz
    Anonymous:
    I'm really surprised that nobody has coughed up the obvious replacement for the first function:

    public bool IsFileReadOnly(int attributes)
    {
      bool retVal = false;

      if ((attributes % 2) == 1)
      {
        retVal = true;
      }

      return retVal;
    }


    There's no (conceptual) bitwiseness to the modulo operator, right?

    WTF, why not

    return ((attributes % 2) == 1;

    ?

  • (cs)

    It took a while, until I 'understood' what that man tried to do. It's like climbing up the Empire State Building on the outside, just to use the elevator down to the cellar, where we wanted to go in the first place.

    Sometimes I'm so glad that assembler was the third programming language I learned.

  • crlf (unregistered) in reply to Jehos
    Jehos:

    To keep going with this theme, the other checks should have had (-64, -31, -16, -8, -4, -2), (-64, -32, -15, -8, -4, -2), etc.  That would give you a nice long ugly way to tell what the bit at a certain position was set to, because you'd get back either a 1 or a 0.  Of course, all of these checks would fail if attributes was ever > 128.

    <br>
     *Ahem*. Everyone seems to be missing the fact that attributes is an int. Not a char. Not an unsigned char. The code if fucked up beyond belief for any value passed in greater than or equal to 64.
  • Anon-e-Moose (unregistered) in reply to Charles Nadolski

    <font size="2">I think you're forgetting the other scenario-- Those of use with no formal training who can code with the best of them.</font>

  • (cs) in reply to diGriz

    I respect this website's privacy policies, I really do, but I think the time has come: Alex, you need to publish the name of the programmer who wrote this.  Not so we can laugh at him; not so we can avoid ever, ever, ever hiring him, but so we can go to his house, drag him out into the street, rip off both his arms, and beat him to death with the soggy ends.

    I don't think that's unreasonable, do you?

  • christophe (unregistered) in reply to Charles Nadolski

    "And no, I didn't even bother to check if it really worked." That's what is really bad with ugly code : we feel entitled to correct the poor newbie without any concern about the correctness of our own solution. How can we expect them to follow us toward software quality if we scathingly substitute a working piece of crap by a clean, factored code that wouldn't even compile ?

    I think any piece of code given without its tests is a good candidate for the wtf contest.

  • Holland (unregistered) in reply to DT
    Anonymous:
    Anonymous:
    I'm really surprised that nobody has coughed up the obvious replacement for the first function:

    public bool IsFileReadOnly(int attributes)
    {
      bool retVal = false;

      if ((attributes % 2) == 1)
      {
        retVal = true;
      }

      return retVal;
    }


    There's no (conceptual) bitwiseness to the modulo operator, right?

    WTF, why not

    return ((attributes % 2) == 1;

    ?

    But what I really like about the modulo solutions is that you can test non-existant bits really easily - eg:

    (attributes % 6)

    easily allows you to check the  2.5849625025'th bit of 'attributes'

  • anonymous (unregistered) in reply to christophe

    "I think any piece of code given without its tests is a good candidate for the wtf contest."

     

    I agree entirely, nothing p*ss*s me off more then untested code I need to work with that "should just work".

    There is no excuse for not testing code, this sometimes gets you into more hot water when working in a development team then doing something stupid.

    There is such an elitism between programmers, maybe if this guy had been given the chance to ask people around him the best approach to the solution it may not have turned out this way.

    Was there any peer review on the code before it ended up on this site?

    The code is a WTF, but not testing is up there too.

     

  • (cs)

    The use of enums and a sealed class would have removed all the need for that crap code!
    [Flags]
    public enum FileStateList
    {
        None = 0,
        ReadOnly = 1,
        Hidden = 2,
        System = 4,
        VolumeId = 8,
        Directory = 16,
        Archived = 32,
        Any = 64
    }   

    public sealed class FileState
    {
        private FileStateList flag;
           
        public FileState(FileStateList flag)
        {
            this.flag = flag;
        }
       
        public bool ReadOnly
        {
            get { return Contains(FileStateList.ReadOnly); }
        }
       
        public bool Hidden
        {
            get { return Contains(FileStateList.Hidden); }
        }
       
        public bool System
        {
            get { return Contains(FileStateList.System); }
        }
       
        public bool VolumeId
        {
            get { return Contains(FileStateList.VolumeId); }
        }
       
        public bool Directory
        {
            get { return Contains(FileStateList.Directory); }
        }
       
        public bool Archived
        {
            get { return Contains(FileStateList.Archived); }
        }
       
        public bool Any
        {
            get { return Contains(FileStateList.Any); }
        }
       
        private bool Contains(FileStateList value)
        {
            return (this.flag & value) == value;
        }
       
       
    }

    calls
    ====

    1. FileState fs = new FileState((FileStateList)10);
    2. FileState fs = new FileState((FileStateList.ReadOnly | FileStateList.Hidden));
      3.
         FileStateList flag = FileStateList.ReadOnly | FileStateList.Hidden;
         FileState fs = new FileState(flag);



  • ASDF (unregistered) in reply to b1xml2

    b1xml2:
    The use of enums and a sealed class would have removed all the need for that crap code!
    [Flags]
    public enum FileStateList
    {
        None = 0,
        ReadOnly = 1,
        Hidden = 2,
        System = 4,
        VolumeId = 8,
        Directory = 16,
        Archived = 32,
        Any = 64
    }   

    public sealed class FileState
    {
        private FileStateList flag;
           
        public FileState(FileStateList flag)
        {
            this.flag = flag;
        }
       
        public bool ReadOnly
        {
            get { return Contains(FileStateList.ReadOnly); }
        }
       
        public bool Hidden
        {
            get { return Contains(FileStateList.Hidden); }
        }
       
        public bool System
        {
            get { return Contains(FileStateList.System); }
        }
       
        public bool VolumeId
        {
            get { return Contains(FileStateList.VolumeId); }
        }
       
        public bool Directory
        {
            get { return Contains(FileStateList.Directory); }
        }
       
        public bool Archived
        {
            get { return Contains(FileStateList.Archived); }
        }
       
        public bool Any
        {
            get { return Contains(FileStateList.Any); }
        }
       
        private bool Contains(FileStateList value)
        {
            return (this.flag & value) == value;
        }
       
       
    }

    calls
    ====
    1. FileState fs = new FileState((FileStateList)10);
    2. FileState fs = new FileState((FileStateList.ReadOnly | FileStateList.Hidden));
    3.
       FileStateList flag = FileStateList.ReadOnly | FileStateList.Hidden;
       FileState fs = new FileState(flag);



    http://msdn.microsoft.com/library/default.asp?url=/library/en-us/cpref/html/frlrfSystemIOFileAttributesClassTopic.asp

  • (cs) in reply to ASDF

    [Defense mode]

    I think I see what went wrong here.

    The person who made it might have been a VB6 programmer (explaining the a=a-1 type lines). In VB6 bitwise operations work with the AND and OR statements. The same ones used in IF/THEN lines. PErhaps this person tried using && and || as bitwise statements, found out it didn't work, needed to finish the job, built it this way, and is now kicking himself in the head for not taking some time to figure out bitwise ops in C#.

    [/Defense mode]

    Drak

  • Anon (unregistered)

    what about

    bool isFileReadOnly(int attributes) {
      return attributes & 1;
    }

  • LG (unregistered) in reply to Charles Nadolski

    Are you trying to say people with GED's and no degree's are stupid?


    It's a filtering effect: anybody, both competent and intelligent people and harmful morons, can be uneducated, while the process of getting a technical degree is expected to ensure that people like today's featured fool are rejected or improved to a decent level.
  • deepspace (unregistered)

    Well, this guy must have known one of my Univercity teachers...

    One, we did a project and used some simple bitwise opp's. We mailed the guy the code for review, and then we had to come by to explain to him why we used bitwise opps... He honestly didn't understand how they could be usefull...

    Oh well, that was probably one of the reasons to quit school and start making money :P

  • (cs) in reply to anonymous
    Anonymous:

    "I think any piece of code given without its tests is a good candidate for the wtf contest."

     

    I agree entirely, nothing p*ss*s me off more then untested code I need to work with that "should just work".

    There is no excuse for not testing code, this sometimes gets you into more hot water when working in a development team then doing something stupid.

    There is such an elitism between programmers, maybe if this guy had been given the chance to ask people around him the best approach to the solution it may not have turned out this way.

    Was there any peer review on the code before it ended up on this site?

    The code is a WTF, but not testing is up there too.

     



    i think you both are off the point
    why bother testing a code, which, compared to the original, is simpler, cleaner, and equally (if not more) broken from the start?

Leave a comment on “It's a Bit Defective”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article