Return to Answer

added 1068 characters in body

Source Link

edited Feb 3, 2014 at 3:18

Jamal

34.9k
13
133
237

You appear to be doing everything in main while just branching into single-character procedures. Consider renaming them to specify their purpose, while separating them from main with proper indentation and such. I cannot tell what main should be doing by itself.
You appear to be doing everything in main while just branching into single-character procedures. Consider renaming them to specify their purpose, while separating them from main with proper indentation and such. I cannot tell what main should be doing by itself. I can just see that it prompts for user input and operates the two loops with its procedures.

Since prompt is done first, consider defining it above main instead. The end of the program should just display the result(s).

Structure

The register initializations should be placed above main for better visibility and maintainability.

The loop labels barely stand out, and this really hurts structure. The loop procedures are all crammed together and L1 looks like just another procedure. You could at least rename L1 and L2 to Loop1 and Loop2 respectively.

The procedures should be properly indented to help them stay isolated.

P:, for instance, hardly stands out:

  P:  add $t1, $t1, $t5   # increment i

  move $t2, $t6

  bgt  $t1, $t0, E    
  j L2

All of that could should be indented together and with P: on its own line:

  P:  
      add $t1, $t1, $t5   # increment i

      move $t2, $t6

      bgt  $t1, $t0, E    
      j L2

You appear to be doing everything in main while just branching into single-character procedures. Consider renaming them to specify their purpose, while separating them from main with proper indentation and such. I cannot tell what main should be doing by itself.

You appear to be doing everything in main while just branching into single-character procedures. Consider renaming them to specify their purpose, while separating them from main with proper indentation and such. I cannot tell what main should be doing by itself. I can just see that it prompts for user input and operates the two loops with its procedures.

Since prompt is done first, consider defining it above main instead. The end of the program should just display the result(s).

Structure

The register initializations should be placed above main for better visibility and maintainability.

The loop labels barely stand out, and this really hurts structure. The loop procedures are all crammed together and L1 looks like just another procedure. You could at least rename L1 and L2 to Loop1 and Loop2 respectively.

The procedures should be properly indented to help them stay isolated.

P:, for instance, hardly stands out:

  P:  add $t1, $t1, $t5   # increment i

  move $t2, $t6

  bgt  $t1, $t0, E    
  j L2

All of that could should be indented together and with P: on its own line:

  P:  
      add $t1, $t1, $t5   # increment i

      move $t2, $t6

      bgt  $t1, $t0, E    
      j L2

Source Link

created Feb 3, 2014 at 1:55

Jamal

34.9k
13
133
237

Procedures

You appear to be doing everything in main while just branching into single-character procedures. Consider renaming them to specify their purpose, while separating them from main with proper indentation and such. I cannot tell what main should be doing by itself.

Comments

The side comments here seem redundant:
```
  # Load variables into registers
  lw  $t1, i  # $t1 = i
  lw  $t2, k  # $t2 = k
  lw  $t3, p  # $t3 = p
  lw  $t5, c  # $t5 = c
  lw  $t6, d  # $t6 = d
```
The top comment describes this block clearly enough, whether or not the reader already knows that lw means load word. The side comments don't add anything else here.
This comment is also redundant:
```
  syscall       # call operating sys
```
Unless there are some special circumstances to note, such comments do not add anything important. Readers familiar with MIPS commands will already know what this means. You may just mention this at the top of the program if you'd like for anyone to know what this means. Putting it with every call just clutters your code.

This, on the other hand, doesn't state the obvious:
```
  add $t2, $t2, $t5   # increment k
```
Without the comment, all I can tell is that this is equivalent to $t2 += $t5, and not that it increments k. These are situations where commenting on separate commands is helpful.

Overall, this is just very hard to follow. If there are bugs in here, it would be difficult to isolate them. This, in turn, hurts maintainability. Comments alone can't help with this, either. The overall structure must be sound, otherwise fixing problems is just a matter of picking apart separate lines, one-by-one.