Pascal programming help

Question

I posted this earlier and it got closed because I did not show my try at coding it, so here is the question:

SECTIONS
$160 = section 1
$220 = section 2
$280 = section 3
$350 = section 4
$425 = section 5

Develop pseudocode that accepts as input the name of an unspecified number of masqueraders who each have paid the full cost of their costume and the amount each has paid.

A masquerader may have paid for a costume in any of the five sections in the band. The algorithm should determine the section in which the masquerader plays, based on the amount he/she has paid for the costume. The algorithm should also determine the number of masqueraders who have paid for costumes in each section.

The names of persons and the section for which they have paid should be printed. A listing of the sections and the total number of persons registered to play in each section should also be printed, along with the total amount paid in each section.

Here is my try at it: *Note this is programmed in Pascal, and I need help fixing it up and finishing it. Please help and thanks again.

program Masqueraders;

uses
  WinCrt;  { Allows Writeln, Readln, cursor movement, etc. }

const
   MAX = 5; {this determine the amount of masquarader entered}
Type
  listname = Array[1..MAX] of string;
  listsect = Array[1..MAX] of string;
var
 names : listname;
 sections : listsect;
 i, amount, TotalMas, TotalAmt, c1, c2, c3, c4, c5, amt1, amt2, amt3, amt4, amt5 :     integer;

begin

 amount := 1;
 while amount <> 0 do
 begin

      i := i + 1;
      readln(names[i]);
      readln(amount);

      if(amount = 160) then
      begin

                c1 := c1 + 1;  {Count the number of persons for section 1}
                amt1 := amt1 + amount; {accumulate the amount for section 1}
                sections[i] := 'Section 1';
      end;

      if(amount = 220) then
      begin

                c2 := c2 + 1;  {Count the number of persons for section 1}
                amt2 := amt2 + amount; {accumulate the amount for section 1}
                sections[i] := 'Section 2';
      end; {end the IF for section 2}

      if(amount = 280) then
      begin

                c3 := c3 + 1;  {Count the number of persons for section 1}
                amt3 := amt3 + amount; {accumulate the amount for section 1}
                sections[i] := 'Section 3';
      end; {end the IF for section 3}

      if(amount = 350) then
      begin

               c4 := c4 + 1;
               amt4 := amt4 + amount;
               sections[i] := 'Section4';
      end; {end If for section 4}

      if (amount = 425) then
      begin

               c5 := c5 + 1;
               amt5 := amt5 + amount;
               sections[i] := 'Section5';


  end;{end the while loop}

  TotalMas := c1 + c2 + c3;
  TotalAmt := amt1 + amt2 + amt3;


  writeln('Name                    Section');  {Heading for the output}
  for i := 1 to MAX do
  begin

       write(names[i]);
       writeln('                    ',sections[i]);

  end;


  writeln('Section 1: ');
  write('Masquader: ', c1);
  write('Amount: ', amt1);



  writeln('Total Number of Masquarader: ', TotalMas);
  writeln('Total Amount Paid by masquarader: ', TotalAmt);

end; end.

In short, it should accept an undefined number of people and assign them to their respective sections based on the amount of money they entered, then calculate the number of people in each section. This is my current output:

Name John Money=160 Section 1

Name Keith Money=220 Section John

This is what I want:

Name John Money=160 Section1

Name Keith Money=220 Section2

Answer 1

I'm not really familiar with Pascal so I can't tell you how to fix this but one issue I notice is that your code seems to violate the "DRY" rule: Don't Repeat Yourself. The code inside each if amt = xxx block looks almost exactly the same, so is there a way you can write that code once, and then call your code with different parameters each time? Something so that you're not rewriting the same code five times.

Answer 2

Here's are a few hints to improve your code:

• Do you think there might be a way to print "Section {i}" in your final loop without looking it up using sections[i], thereby negating the need for the array completely?

• How could you build this so that adding a section 6 wouldn't require modifications to your code?

• listname and listsect are awfully similar, what modifications to your code could you do to eliminate the need to have two identical definitions?

• What happens if the user enters 0 for the amount the third time they are prompted?

Note: one of these hints should point you directly to the source of your problem.

Answer 3

Here are the problems I see:

1. The requirements say an "unspecified number of masqueraders" but you have set the max to 5. You can't store the list of names in a fixed size array if there are possibly going to be more than 5 masqueraders. As is, the app may either crash or corrupt memory (depending on the version of Pascal you're using) when user enters the 6th masquerader. If you are allowed to print the masquerader's Name and Section immediately after it is input, then you should print that right after the user inputs the Name and Amount (not after the input while loop). However, if it is required that you print the list of Names and Sections after all input, then you need to store the Name+Section in a variable length data structure like a linked list or even simpler a string.

2. The variables c1 to c5 and amt1 to amt5 would be better declared as two arrays (c and amt) of 1..MAX instead of 10 variables. When user inputs the amount, use it determine the section number which then can be used as the index into the c and amt arrays.

3. Not sure what implementation of Pascal you're using but it would be safer to initialize all variables before use otherwise they could contain unpredictable values.

4. The sections array is unnecessary. All it ends up containing is the title of the section which doesn't need to be calculated or stored.

5. If you used a repeat/until loop instead of a while loop, it would avoid the slightly kludgy "amount := 1" initialization at the top to enter the while loop. The repeat/until would be until Amount = 0.

6. TotalMas is only adding c1 to c3 (what about c4 and c5)?

7. TotalAmt is only adding amt1 to amt3 (what about amt4 and amt5)?

8. The for-loop to print the names and section is completely wrong, see points 1 and 4.

Hope this helps and let me know if you need clarification on any points.

Answer 4

My gripes with this code:

1) The variable names stink. Virtually all of them are way too short, they do not communicate what they do.

2) The wrong loop control structure is used here.

3) I see virtually identical code repeated 5 times. This is simply crying out for arrays.

4) The comments explain what you are doing, not why you are doing it. Half of them are virtually completely useless, if you're going to comment an end you should simply comment it with the identity of whatever it's an end to. end; //if I see only one comment that even tries to explain why and you've got it wrong as you're mixing up people with sections.

5) Your sections array accomplishes nothing useful. The assignments to the values are always fixed and thus there is no reason to store them at all--if you need the labels you can create them at print time.

6) You are assuming there are only going to be 5 people. That's not given in the problem--there's 5 sections.