views:

179

answers:

9

I'm reviewing some code from one of our contractors:

if (userLists != null)
{
    Int32 numberOfItems = userLists.Count;
    if ((numberOfItems & 1) == 1)
    {
        var emptyList = new tblList();
        userLists.Add(emptyList);
    }
}

Now, I'm trying to understand this, so, can someone please confirm with me if I have got this right or not?

  1. Do we have an instance of a userList?
  2. Yes. Get a the number of items in the userLists object.
  3. if (the number of items equals 1 (Yes/true) AND 1 equals 1 (yes/True) ) AND that result equals 1 (Yes/True), then add an empty list object to the list.
  4. Else: nothing.

If so (W.T.FFFFFFFFFFFFFFFFFFFF !!!!!!!!!!), that can be refactored to

if (numberOfItems == 1)
{
..
}

but even that's crap because I don't want a list with 'empty' items.

So did I read that piece of code correctly?

Oh, one more sigh at the use of Int32 vs Int :( (but I digress).

+3  A: 

Checking for odd's, Can also do i % 2 != 0

I would suggest looking at BitMasks they can be pretty handy, but not in the code in your question, I would prefer the modulous if you needed to do even/odd.

    static void Main(string[] args)
    {
        for (int i = 0; i < 100; i++ )
            Console.WriteLine( i & 1);
        Console.ReadLine();
    }


1
0
1
0
1
0

And for the heck of it, here's some extension methods

   class Program
    {
        static void Main(string[] args)
        {
            List<int> ints = new List<int>();

            for (int i = 0; i < 100; i++)
            {
                Console.WriteLine("Mod: {0}", i % 2 );
                Console.WriteLine("BitWise: {0}", i & 1 );

                ints.Add(i);
                Console.WriteLine("Extension: {0}", ints.IsEven() );
            }
            Console.ReadLine();
        }
   }

    public static class ListExtensions
    {
        public static bool IsEven<T>(this ICollection<T> collection)
        {
            return (collection.Count%2) == 0;
        }

        public static bool IsOdd<T>(this ICollection<T> collection)
        {
            return (collection.Count%2) != 0;
        }
    }
Chad Grant
aggreed mate! i prefer modulo, for sure!
Pure.Krome
+1, Using mod rather than bitwise AND makes the intent of the code much more obvious when you're doing odd/even checks.
LukeH
+6  A: 

Bitwise-ANDing a number with 1 checks whether the number is odd or even (returns 1 if odd). What this code is doing is making sure the list has an even number of items by adding another item if there is an odd number of items.

yjerem
A: 

(numberOfItems & 1) == 1

is more like numberOfItems%2 != 0

leiz
A: 

for the (numberOfItems &1)==1, that's a bitwise AND. It seems to be checking if numberOfItems is odd, and adding an empty list if it is.

T.R.
A: 
((numberOfItems & 1) == 1)

& with 1 tests the 0th bit. For integer data types all odd values have the 0th bit set and all the even values have it cleared. The code above effectively tests for the value being odd.

sharptooth
A: 

Could be refactored to be more understandable/maintainable:

if (userLists != null)
{    
    EnsureListHasAnEvenNumberOfItems(userLists);
}
Mitch Wheat
Alternatively: bool ListHasOddLength = userLists.Count%2==1; if (ListHasOddLength) userLists.add(new tblList()); //Abstraction without having to jump around to different methods
T.R.
A: 

Is zero elements ok?

x = (0 & 1) then (x == 1) is false...

I think you should ask your contractors to comment their code more.

JH
+6  A: 

The & is what is called a bitwise operator. Whereas the operator && tests two boolean values:

TRUE && FALSE => FALSE
TRUE && TRUE => TRUE

the & operator can work on integer values:

  00101101 (45)
& 01011011 (91)
---------------
= 00001001 (9)

Each bit has the boolean operation (and) done on it. So in the case of your code sample, it is asking "is the last bit a 1?" - that is to say "is it odd?" For instance, if the number is 23:

  00010111 (23)
& 00000001 (1)
---------------
= 00000001 (1)

So it adds to the list because 1 == 1. But if the number was 22:

  00010110 (22)
& 00000001 (1)
---------------
= 00000000 (0)

So it does not add to the list.

Smashery
Pure.Krome
Right you are. I have fixed it. Cheers!
Smashery
Can you use it for other purposes than check if a number is odd?Useful purposes :D
CasperT
Absolutely! Bitwise operators (
Smashery
+2  A: 

Since we're being silly ...

public static class Extensions
{
    public static bool IsEven(this Int32 integer)
    {
        return (integer % 2 == 0);
    }
}

Let's you do ...

numberOfItems.IsEven()
JP Alioto
fluent!! nice :)
Pure.Krome
Gordon Mackie JoanMiro