I have been working on a certain task these days and after several hours of torture, I have done it! However, I believe the code that I have "crafted" is quite difficult to be understood and to be honest, I wouldn't be able to understand it if I wasn't the one who made it. Anyways, this is my code. I would like to receive some advice about what I could do to make it a little bit more simple and, by any chance, shorter:

3 Answers
3

I think this is suggesting roughly (maybe exactly) the same technique as @harold does in his answer, but I'm not entirely certain that I understand his answer, so I can't say with certainty whether it's the same or not.

I'd consider two facts in coming to an answer. First, that where two bits are identical, we don't need to do anything to swap them--i.e., swapping a 1 with a 1 or a 0 with a 0 doesn't change anything.

Therefore, we only need to do anything where the bits are not equal to each other. In this case, we have to flip both bits--i.e., the one that was a 0 has to become a 1, and the one that was a 1 has to become a 0.

At least to me, that immediately brings an exclusive-or to mind. An exclusive-or produces a 1 if one input or the other (but not both) is a 1. Looking at it slightly differently, it produces a 1 if the bits are not equal to each other, and a 0 if they are equal to each other. Looked at from yet a slightly different angle, an XOR with 0 leaves a bit unchanged, and an XOR with 1 always flips the bit (changes a 0 to a 1, or a 1 to a 0).

With that in hand, the job becomes fairly simple: we start by XORing the bits in the two ranges. This gives us 0s where they contain identical bits (meaning we want to leave them unchanged). It gives us 1s where they contain differing bits (meaning we want to flip the bits in both ranges).

We then XOR that mask with the bits in each range. That leaves the bits in each range unchanged where they were identical to start with, and flips the bits in each range where they were different.

For example, let's consider swapping the first 4 bits of a byte with the second 4 bits:

The only things that change from one of these to the next are the string you display and the identity of the variable you're reading. This is an ideal candidate for being turned into a function. The function needs to read a number (just as you've done it above) and return the value when it's successful. Then the code in main will just call that function for each of the numbers it needs to read:

It's probably possible to avoid having to pass the string as well--you'd pass the variable itself, and figure out the name of that variable using reflection to build your string. That takes some rather more advanced use of .NET though, and I wouldn't advise it for now.

When it comes to finding which range is first:

if (p>q)
{
num1 = q;
num2 = p;
}
else
{
num1 = p;
num2 = q;
}

I think I'd just use p and q, but swap them if they're out of order:

if (p>q) {
temp = p;
p = q;
q = temp;
}

(...and wish you were using C++, so you didn't have to re-implement basic operations like this for the 4 billionth time).

I've already advised a complete rewrite of the swapping code itself, so I guess there's not much point in reviewing that part of your code.

I tried to answer when this question was on SO, but you removed and reposted it here.. so here's a non-code-review. This is not a comment on your code, but a suggestion for an entirely different approach.

That's much more complicated then it has to be.

Look into delta swaps. delta(x, m, d) takes an input x, a mask m and a distance d (the delta), and swaps non-overlapping pairs of bits such that one of the pair is indicated by the mask and the other of the pair is d places towards the most significant bit.

It sounds more complicated than it is: (this is pseudo code, and not tested, you can look up the real definition in The Art of Computer Programming 4A, Bitwise tricks and techniques)

If you recast your problem in terms of delta, the only trouble is getting the mask and the distance. The distance is just abs(p - q). Generating a mask from some position to an other is a common problem (it has been asked on SO several times) and not too hard.

This same code is repeated several times almost verbatim. You should try to avoid that (it's called Don't Repeat Yourself, or DRY for short) and instead write a method to encapsulate this. For example: