Comment On It's a Bit Defective

This is so beyond retarded.

Horror! Horror! [:|]

Unless this person getting paid by the number of lines (s)he produce...  [;)]

 

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...".

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

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.

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 :*(

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.

[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!

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;
  }

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.

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.

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

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

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

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.

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.

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

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;

}

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..

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).

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)

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).

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.

I just love the title!!!

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.......

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?

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?

Charles Nadolski:

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

n--

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<<=1;
  }
  return (attributes<0)?1:0;
}

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



return ((FileAttributes)attributes & FileAttributes.ReadOnly) == FileAttributes.ReadOnly;



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))

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:

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

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!

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:

public bool IsFileReadOnly(int attributes) {
  for(int i=0; i
      attributes<<=1;
  }
  return (attributes<0)?1:0;
}

I think you win :) *golf claps*  Good show!

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.

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;

}

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;

?

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.

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.

I think you're forgetting the other scenario-- Those of use with no formal training who can code with the best of them.

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?

"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.

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'

"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.

 

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);

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

[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

what about



bool isFileReadOnly(int attributes) {

  return attributes & 1;

}

> 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.

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

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?