455,127 Members | 1,222 Online Need help? Post your question and get tips & solutions from a community of 455,127 IT Pros & Developers. It's quick & easy.

# How can I make this code less ugly?

 P: n/a I have written a sorting program, to try to understand how heapsort works. Unfortunately, considering it's supposed to help my understanding, one line of it has got very ugly and so I was wondering if anyone had any advice on how to make it clearer. The code is C++, but I am rashly cross-posting to comp.lang.c as the part I am having difficulty with is the same as C and I thought the peoplr there might also be able to help. You can make it a proper C program by replacing the definition of n with a #define. I'm aware that the sort is not actually a heapsort, but I think I know where I'm going there. I also appreciate that it is only a "toy" program at present and if I was going to put it to proper use I would be better with a function pointer for the comparison. So here is the code: #include #include #include
10 Replies

 P: n/a gw****@aol.com wrote: The code is C++ So here is the code: #include #include #include

 P: n/a On Mon, 10 Jul 2006 gw****@aol.com wrote: I have written a sorting program, to try to understand how heapsort works. Unfortunately, considering it's supposed to help my understanding, one line of it has got very ugly and so I was wondering if anyone had any advice on how to make it clearer. [snip] y = 0; while (y < h && (b[z2=2*y+2,z1=2*y+1] < EMPTY || b[z2] < EMPTY)) { How about this: for (y = 0, z1 = 1, z2 = 2; y < h && (b[z1] < EMPTY || b[z2] < EMPTY); z1 = 2 * y + 1, z2 = 2 * y + 2) { Tak-Shing Jul 10 '06 #3

 P: n/a Noah Roberts wrote: gw****@aol.com wrote: >The code is C++ >So here is the code:#include #include #include const int n = 11;const int h = (n-1)/2; The initialiser for h is not a constant expression in C. -- Simon. Jul 11 '06 #4

 P: n/a gw****@aol.com wrote: while (y < h && (b[z2=2*y+2,z1=2*y+1] < EMPTY || b[z2] < EMPTY)) { if(b[z1] <= b[z2]) { b[y] = b[z1]; y = z1; } else { b[y] = b[z2]; y = z2; } } while (y < h && (z1=2*y+1, z2=2*y+2, b[z1] < EMPTY || b[z2] < EMPTY)) if (b[z1] <= b[z2]) b[y] = b[z1], y = z1; else b[y] = b[z2], y = z2; or while (y < h && (z = 2*y, b[z+1] < EMPTY || b[z+2] < EMPTY)) if (b[z+1] <= b[z+2]) b[y] = b[z+1], y = z+1; else b[y] = b[z+2], y = z+2; or while (y < h && (b[2*y+1] < EMPTY || b[2*y+2] < EMPTY)) z = 2*y + (b[2*y+1] <= b[2*y+2] ? 1 : 2), b[y] = b[z], y = z; or while (y < h && (z = 2*y, b[z+1] < EMPTY || b[z+2] < EMPTY)) z += b[z+1] <= b[z+2] ? 1 : 2, b[y] = b[z], y = z; -- Dietmar Jul 13 '06 #5

 P: n/a Dietmar Schindler wrote: gw****@aol.com wrote: while (y < h && (b[z2=2*y+2,z1=2*y+1] < EMPTY || b[z2] < EMPTY)) { if(b[z1] <= b[z2]) { b[y] = b[z1]; y = z1; } else { b[y] = b[z2]; y = z2; } } while (y < h && (z1=2*y+1, z2=2*y+2, b[z1] < EMPTY || b[z2] < EMPTY)) if (b[z1] <= b[z2]) b[y] = b[z1], y = z1; else b[y] = b[z2], y = z2; or while (y < h && (z = 2*y, b[z+1] < EMPTY || b[z+2] < EMPTY)) if (b[z+1] <= b[z+2]) b[y] = b[z+1], y = z+1; else b[y] = b[z+2], y = z+2; or while (y < h && (b[2*y+1] < EMPTY || b[2*y+2] < EMPTY)) z = 2*y + (b[2*y+1] <= b[2*y+2] ? 1 : 2), b[y] = b[z], y = z; or while (y < h && (z = 2*y, b[z+1] < EMPTY || b[z+2] < EMPTY)) z += b[z+1] <= b[z+2] ? 1 : 2, b[y] = b[z], y = z; Thanks for your suggestions. I think the first option may be the one to go for - setting both the values of z1 and z2 before mentioning b. Either that, or miss out the z's completely - more like your second option. Jul 13 '06 #6

 P: n/a gw****@aol.com wrote: Dietmar Schindler wrote: >gw****@aol.com wrote: >> while (y < h && (b[z2=2*y+2,z1=2*y+1] < EMPTY || b[z2] < EMPTY)) { if(b[z1] <= b[z2]) { b[y] = b[z1]; y = z1; } else { b[y] = b[z2]; y = z2; } } while (y < h && (z1=2*y+1, z2=2*y+2, b[z1] < EMPTY || b[z2] < EMPTY)) if (b[z1] <= b[z2]) b[y] = b[z1], y = z1; else b[y] = b[z2], y = z2;or while (y < h && (z = 2*y, b[z+1] < EMPTY || b[z+2] < EMPTY)) if (b[z+1] <= b[z+2]) b[y] = b[z+1], y = z+1; else b[y] = b[z+2], y = z+2;or while (y < h && (b[2*y+1] < EMPTY || b[2*y+2] < EMPTY)) z = 2*y + (b[2*y+1] <= b[2*y+2] ? 1 : 2), b[y] = b[z], y = z;or while (y < h && (z = 2*y, b[z+1] < EMPTY || b[z+2] < EMPTY)) z += b[z+1] <= b[z+2] ? 1 : 2, b[y] = b[z], y = z; Thanks for your suggestions. I think the first option may be the one to go for - setting both the values of z1 and z2 before mentioning b. Either that, or miss out the z's completely - more like your second option. How about moving the test into a separate function as well while (myCondition()) { // option 1 code. } Jul 13 '06 #7

 P: n/a Simon Biber wrote: Noah Roberts wrote: gw****@aol.com wrote: The code is C++ So here is the code: #include #include #include

 P: n/a * Noah Roberts: Simon Biber wrote: >Noah Roberts wrote: >>gw****@aol.com wrote:The code is C++So here is the code:#include #include #include

 P: n/a Noah Roberts wrote: Simon Biber wrote: >Noah Roberts wrote: >>gw****@aol.com wrote:The code is C++So here is the code:#include #include #include

 P: n/a Noah Roberts wrote: > Simon Biber wrote: Noah Roberts wrote: gw****@aol.com wrote: >The code is C++ > >So here is the code: >#include >#include >#include Nope, the code is C. The code you quoted is valid in both C and C++. Those headers don't exist in C++. OT, but yes they do. They are deprecated, but perfectly standard. The headers , , and

### This discussion thread is closed

Replies have been disabled for this discussion. 