patterncppMinor
Take all the bits in a number and shift them to the left end
Viewed 0 times
shiftleftnumberthealltakebitsendandthem
Problem
I wrote this program which takes all the bits in a number and shifts them to the left end (for example: 01010110 --> 11110000). It's and exercise from a book. It works, but it seems to be very wasteful to use integers (I only need 8 bits).
Can you tell me if the code is proper or not? Is the algorithm good?
Can you tell me if the code is proper or not? Is the algorithm good?
#include
void print_bit(const int bit)
{
for(int i = 0x80; i != 0; i >>= 1)
{
if((bit & i) != 0)
std::cout >= 1)
{
if((bit & i) == 0)
lefted <<= 1;
}
return lefted;
}
int main()
{
int bit = 11;
print_bit(bit);
print_bit(set_bits(bit));
return 0;
}Solution
It appears the correct results are given, which is good. I do see some miscellaneous things, though.
-
The naming and execution in
-
This:
could become a single-line ternary statement:
-
This:
could have curly braces (to allow any additional code for the body):
-
I'd probably make your "magic numbers" (such as
-
`std::cout
-
The naming and execution in
main() seem confusing. Try something like this:int main()
{
// the name "bit" is misleading; you're using an integer
// name it something more accurate (this is just an idea)
int original = 11;
// why not return the final (shifted) integer?
// sticking set_bit() into print_bit() is hard to read
int final = set_bits(original);
// print both integers
print_bit(original);
print_bit(final);
}-
This:
if((bit & i) != 0)
std::cout << "1";
else std::cout << "0";could become a single-line ternary statement:
// read as: (statement is true?) ? (if so, do this) : (if not, do this)
std::cout << ((bit & i) != 0) ? "1" : "0";-
This:
if((bit & i) == 0)
lefted <<= 1;could have curly braces (to allow any additional code for the body):
if ((bit & i) == 0)
{
lefted <<= 1;
}-
I'd probably make your "magic numbers" (such as
0x80) constants. This could help provide more context for your code, especially when there's no relevant documentation.-
`std::cout
Code Snippets
int main()
{
// the name "bit" is misleading; you're using an integer
// name it something more accurate (this is just an idea)
int original = 11;
// why not return the final (shifted) integer?
// sticking set_bit() into print_bit() is hard to read
int final = set_bits(original);
// print both integers
print_bit(original);
print_bit(final);
}if((bit & i) != 0)
std::cout << "1";
else std::cout << "0";// read as: (statement is true?) ? (if so, do this) : (if not, do this)
std::cout << ((bit & i) != 0) ? "1" : "0";if((bit & i) == 0)
lefted <<= 1;if ((bit & i) == 0)
{
lefted <<= 1;
}Context
StackExchange Code Review Q#30740, answer score: 3
Revisions (0)
No revisions yet.