Breaking up complex code into functions
Andrew Bennett <andrew.bennett@unsw.edu.au>, COMP1511 18s1 Course Forum
Breaking up a very complicated piece of code into functions
Student:
Andrew
Could you give me an example of what you mean?
Sure.
Here's an excerpt of something I submitted but I wasn't sure on whether or not it would be considered good style. I made some short notes on what it did related to the exercise, but there were several ways in which I could have made it more readable, at the expense of making the submission very long.
I'm not sure if in this case I should just abandon the approach of using complicated inequalities in if statements, expanding on the names of the variables, or increasing the number of if statements to make it more legible. The inequalities help print a spiral.
15 while(i < (n * n)) { // The loop that ensures a box is printed
16 y = (i / n); // Y is defined by integer division
17 x = (i%n); // X is defined by modular arithmetic
18 if (y == 0 || x == n - 1 || y == n - 1 || (x == 0 && y != (0 || 1))) { // This draws the boundarx spiral loop, to begin the rest of the spiral at 2k
19 printf("*");
20 } else {
21 r = 0; // start r at 0
22 while(r < n){ // (proven on paper) r < n for all r in Z
23 if(((((2 * r < x) && (x < n - 2 - 2 * r) && (y == 2 + 2 * r)) || ((x == n - 3 - 2 * r) && (2 * r + 2 < y) && (y < n - 2 * r - 3)) || // Spiral
24 (((1 + (2 * r) < x) && (x < n - 2 - (2 * r)) && (y == n - 2 * r - 3))) || ((x == 2 * r + 2) && (3 + 2 * r < y) && (y < n - 3 - 2 * r))))) { // Function
25 printf("*");
26 r = n;
27 } else {
28 r++; // Parametric value incrmeent
Andrew:
Oh gosh. I mean absolutely no offense by this, but that code is a tangled mess.
I've fixed up just the indentation, and made no other changes; here's how it looks in gedit (note that the vertical bar is at the 80 character line length cut-off):

It doesn't even fit on my screen x_x
Of course, that font size isn't super realistic -- it's much bigger than I'd use when coding, but given that my coding workflow looks like this:

-- I have 4 windows open in vim side-by-side, so that I can refer to different parts of the code or different files at the same time.
Even if I run my automatic-code-formatting tool across it, it's still very hard to read:

Now, if I put some of the logic into functions -- and to be honest, I don't understand what your code is doing or how it works, and I don't really want to put the effort into understanding it right now :P --

I'm sure there are more meaningful function names that you could have there (I'm just going off the two comments you had).
It would probably also be worth explaining why you do
r = n vs r++, because it's not entirely clear why you're doing
that / how it contributes.
Does that sort of make sense?
Basically, I look at this spiral code now and go, oh yeah, that sort of
makes sense, first check if it's in the outside spiral (easy case), and
then if it's not, if [something] or [something], then it's a point
in the spiral; and otherwise [somehow reset / change the value of n
to move onto the next pixel.
Student:
I'm aware it was not the best, and this certainly cleans it up.
Thank you for the tips, I'll work them into what I make from now on.