HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Bit shifting and masking

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
andmaskingbitshifting

Problem

I was wondering if someone could review this simple bit of code that originally was in C++, but now converted to C# as practice in using bit-shifting and creating a sort of a quasi-array functionality. Consider the partition size as a size of our array which can only hold a specific size limit for said partition. It was assumed that input is always valid, and thus no fail safes were included. The usage of unsigned integers is intentional as we want only positive values.

If you're wondering about the liberal usage of type casting, it is because it appears that C# is far stricter than C++ with respect to data types.

```
using System;

namespace bitShifting
{
class Program
{

uint bitSize, shiftCount, mask, partionSize;

void setValue(ref uint var, uint k, uint i, uint val)
{

bitSize = sizeof(uint) * 8;

partionSize = (uint)(bitSize / k);
shiftCount = partionSize * i;

mask = (uint)~(((1 > (int)shiftCount);
return var;
}

static void Main(string[] args)
{
Program p = new Program();

uint k;
uint value;
uint maxValue;

uint var = 0;

Console.WriteLine("C++ Lab to C#, Practice Using Pass by Ref");

Console.WriteLine("Valid Partition K size are: 1, 2, 4, 8, 16, 32.");
Console.WriteLine("Enter Partition Size");

k = Convert.ToUInt32(Console.ReadLine());

maxValue = (uint)((1 << (int)(sizeof (uint) * 8 / k)) - 1);

if(k == 1)
{
maxValue = 2147483647;
}
Console.WriteLine("Maximal Value for Partion of Size {0} is 0 to {1}", k, maxValue);

bool getValue = true;
for(uint i = 0; i < k; i++)
{
Console.WriteLine("Input a Value");
value = Convert.ToUInt32(Console.ReadLine());

if (k == 1)

Solution

Layout

-
Put your main method on the top, since it tells main purpose of the application. And, C# isn't like C++, you don't have to define the method ahead, before calling them.

class Program
{
    uint bitSize, shiftCount, mask, partionSize;

    static void Main(string[] args);

    void setValue(ref uint var, uint k, uint i, uint val);
    uint getValue(uint var, uint k, uint i);
}


// Even in c++, you can declare a method signature first
 // This part could also go into header file
 void setValue(PUINT var, UINT k, UINT i, UINT val);
 uint getValue(UINT var, UINT k, UINT i);

 int main(int argc, char *argv[]) { /* ... */ }

 // And, implement them later
 void setValue(PUINT var, UINT k, UINT i, UINT val) { /*...*/ }
 uint getValue(UINT var, UINT k, UINT i) { /*...*/ }


-
Mark the class variables and methods static, so you can call them directly.

// Instead of awkward hack/patch like this
Program p = new Program();
p.setValue(...);


-
Declare the local variables as you need them. Except when the assignment is inside a code block, but you also need to use them outside of that.

Naming

  • PascalCasing for any method, class, public property. And, camelCasing for private and local variable.



  • Avoid using keywords as variable name, especially var the most used one.



  • Avoid single letter variable, other than for for loop, lambda expression...



Misc

-
Avoid magic numbers.

// What is 8? numbers of bits in a byte
//var maxValue = (uint)((1 << (int)(sizeof(uint) * 8 / k)) - 1);

const uint BitsPerByte = 8;

//maxValue = 2147483647;
maxValue = int.MaxValue;


You messed up big here...

  • A uint should be capable to store uint.MaxValue and not int.MaxValue.



-
You are mixing up partition size and partition count. With a partition size of 1 (k = 1), you should be able to enter 32 values (BitSize / k = 32 / 1).

// And, not just 1 (`k`) value
for(uint i = 0; i < k; i++)


-
The hard coded conditions k == 1 and getValue are huge red flags!

//  why?
if(k == 1)
{
    maxValue = 2147483647;
}
Console.WriteLine("Maximal Value for Partion of Size {0} is 0 to {1}", k, maxValue);

// hack..
bool getValue = true;
for(uint i = 0; i < k; i++)
{
    Console.WriteLine("Input a Value");
    value = Convert.ToUInt32(Console.ReadLine());

    // more hack
    if (k == 1)
    {
        Console.WriteLine("Value at Index 0 is: {0}", value);
        getValue = false;
    }
    else
    {
        p.setValue(ref var, k, i, value);
    }
}

for(uint i = 0; i < k; i++)
{
    // hack++
    if (getValue)
    {
        value = p.getValue(var, k, i);
            Console.WriteLine("Value At Index {0} is: {1}", i, value);   
    }
}


Let's fix this...

class Program
{
    const int BitsPerByte = 8, BitSize = sizeof(uint) * BitsPerByte;

    static void Main(string[] args)
    {
        Console.WriteLine("C++ Lab to C#, Practice Using Pass by Ref");

        Console.WriteLine("Valid Partition K size are: 1, 2, 4, 8, 16, 32.");
        Console.WriteLine("Enter Partition Size :");
        var size = int.Parse(Console.ReadLine());

        // using 1L (long) for computation to avoid overflow
        var maxValue = (uint)((1L > size * index & mask;
    }
}


EDIT: Explanation on the bit operation :

static void SetValue(ref uint data, int size, int index, uint value)
{
    /*  :: Given that size = 8 and index = 2 (3rd), value = 10000001, data = 10101010 00000000 00000000 00000000
        00000000 00000000 00000000 10000001 // value
        00000000 10000001 00000000 00000000 // value left-shifted by 16 (size * index)

        00000000 10000001 00000000 00000000 // value left-shifted by 16 (size * index)
        10101010 00000000 00000000 00000000 // data
        ----------------------------------- // OR
        10101010 10000001 00000000 00000000 // result */
    data |= value  mask

        10101010 10000001 00000000 00000000 // data
        00000000 00000000 10101010 10000001 // data right-shifted by 16 (size * index)

        00000000 00000000 10101010 10000001 // data right-shifted by 16 (size * index)                
        00000000 00000000 00000000 11111111 // mask
        ----------------------------------- // AND
        00000000 00000000 00000000 10000001 // result */
    return data >> size * index & mask;
}

Code Snippets

class Program
{
    uint bitSize, shiftCount, mask, partionSize;

    static void Main(string[] args);

    void setValue(ref uint var, uint k, uint i, uint val);
    uint getValue(uint var, uint k, uint i);
}
// Even in c++, you can declare a method signature first
 // This part could also go into header file
 void setValue(PUINT var, UINT k, UINT i, UINT val);
 uint getValue(UINT var, UINT k, UINT i);

 int main(int argc, char *argv[]) { /* ... */ }

 // And, implement them later
 void setValue(PUINT var, UINT k, UINT i, UINT val) { /*...*/ }
 uint getValue(UINT var, UINT k, UINT i) { /*...*/ }
// Instead of awkward hack/patch like this
Program p = new Program();
p.setValue(...);
// What is 8? numbers of bits in a byte
//var maxValue = (uint)((1 << (int)(sizeof(uint) * 8 / k)) - 1);

const uint BitsPerByte = 8;

//maxValue = 2147483647;
maxValue = int.MaxValue;
// And, not just 1 (`k`) value
for(uint i = 0; i < k; i++)

Context

StackExchange Code Review Q#135447, answer score: 14

Revisions (0)

No revisions yet.