Stuck on school assignment

[pocku]

Hello. I'm utterly stuck on this one part of my school assignment (for pascal). We are asked to make a calculator like program which calculates integer values only. The program has to read character by character from an input file and if something like an 'a' or # is entered, then an error message will popup telling the user they can't enter that in the input.

The problem I'm having right now is that I can't seem to properly convert the characters to integers and have them add/multiply/subtract/divide up with eachother correctly. If I input an equation with two numbers like: 1+2 or 120/30 or 1000+300 then it works just fine but if I go: 1+2+3 or 1+20+4 or 1+2+3+4 then I get crazy numbers like 1975 or 17245.

This is what my program looks like so far:

program bbb (input,output, infile);

const
   blank = ' ';

var
   infile                                                       : text;
   c                                                            : char;
   eqn, equation                                                : string[20];
   digit1, digit2, digit3, digit4                           : integer;
   previousnotblank, currentnotblank, legalinteger, previousnum : boolean;

{*****************}

procedure convert;

begin
   digit1:=ord(c)-ord('0');
end; { convert }


{****************}

procedure multidigits;

begin
   digit1:= digit1*10;

   digit3:= ord(c)-ord('0');

   digit1:= digit1+digit3;

   previousnum:=legalinteger;

end; { multidigits }

{****************}

procedure multidigits2;

begin
   digit2:= digit2*10;

   digit4:= ord(c)-ord('0');

   digit2:= digit2+digit4;

   previousnum:=legalinteger;

end; { multidigits }

{*****************}

procedure convert2 ;

begin
   legalinteger:= (ord(c)<=ord('9')) and (ord(c)>=ord('0'));

   if c='-' then
   begin
      read(infile,c);

      if (ord(c)<=ord('9')) and (ord(c)>=ord('0')) then
      begin
         digit2:=ord('0')-ord(c);
         previousnum:=legalinteger;
      end;
   end;

  while legalinteger and (c<>'+') do
   begin

      if legalinteger and previousnum then
         multidigits2
      else

         if legalinteger then
           begin

            digit2:=ord(c)-ord('0');
            writeln('convert2: digit2= ',digit2);
            previousnum:=legalinteger;

           end;

      if not eoln(infile) then
         read(infile,c)
      else
         legalinteger:=false;

   end;

      if (ord(c)>ord('9')) or (ord(c)<ord('0')) then
        begin
          writeln('error');

          if (c=' ') then
             writeln("There can't be an operation sign at the end.");
             equation:='notcorrect';
        end;


end; { convert2 }


{****************}

procedure plus;
begin

   read(infile,c);

   currentnotblank:= c <> blank;

   previousnum:=false;

   if not currentnotblank then
        begin
           while (c = blank) and not eoln(infile) do
              begin
              read(infile,c);
              end;
           convert2;
           digit1:=digit1+digit2;

        end;

   if currentnotblank then
      begin
         convert2;
         digit1:= digit1+digit2;
      end;

end; { plus }

{****************}
begin
   assign(infile, 'input.txt');

   writeln('Enter equation: ');
   readln(eqn);

   rewrite(infile);
   write(infile,eqn);
   writeln(infile);

   reset(infile);

   while not eof(infile) do
      begin
         previousnum:=false;

   while not eoln(infile) do
   begin

      read(infile,c);
      currentnotblank:= c<>blank;

      if currentnotblank then
      begin

         if (ord(c)>ord('9')) or (ord(c)<ord('*')) or (c='.') or (c='\') then
            begin
               writeln(' There is an error within the equation ');
               equation:= 'notcorrect';
            end;

         legalinteger:= (ord(c)<=ord('9')) and (ord(c)>=ord('0'));

         if legalinteger and previousnum then
            multidigits
         else
         if legalinteger then
            begin
               convert;
               previousnum:=legalinteger;
            end;

         if (c='+') then
            plus
      else
         if (c='*') then
            multi
      else
         if (c='/') then
            divide
      else
         if (c='-') then
            subtract;

      end;


   end; {while not eoln}

   if equation = 'notcorrect' then
      write('There is no answer.')
   else
      write('The answer is: ',digit1);

    writeln;
    readln(infile);

   end; {while not eof}

end.

I left out the subtract, divide and multiply procedures since they look exactly like the plus procedure.

I think the source of the problem is probably with the while-loop under convert2. With an equation like 1+2+3+4, the program seems to be running smoothly when it reads 1, +, and 2, but once it reads the second +, it doesn't call up the plus procedure and instead tries to convert + into an integer.

Any help would be greatly appreciated!!!

[Duoas]

You should be making better use of functions. For example, convert would be better written as:

function convert( c: char ): integer;
  begin
  convert := ord( c ) - ord( '0' )
  end;

And then used thus: digit1 := convert( c ); Also, you don't need to specify stuff after the program name. Just say: program bbb; In the whole history of Pascal specifying I/O stuff there has only once or twice actually made any difference at all (back when computers were happy with 10K of memory), and never has anyone cared either way. Since you are using a Borland dialect you can just drop it...

You should make your eqn just a little longer, like string[ 80 ] or something, just in case...

The variable currentnotblank is totally superfluous... Just test to see if c <> blank and put then and else branches as needed...

I'm sorry... I'll analyze your code better tomorrow (after I've slept some more). It is pretty scattered and you are using a lot of variables.

This is a bit of a tough assignment for a new programmer --as you know nothing about how to organize an expression reader to handle operator precedence... so you'll have to do without. However, I'll give a hint: a proper calculator expression always has the form: expression ::= number [operator expression] or, as I expect you want it: expression ::= [expression operator] number What that means is that you can expect to read an expression by reading a number first, then (optionally) an operator, and if there was an operator then another expression.

[Duoas]

OK, I've had some time now. Yes, your problem is (mostly) in convert2.

First, a syntax error you should have caught when you tried to compile this: Line 98 should read: writeln('There can''t be an operation sign at the end.'); You can't use " to delineate a string in Pascal.

For the input '1+2':
Before you started the convert2 procedure you had already read '1' and '+', so the procedure read '2' and summed 1 and 2. Then, at eoln, printed the result.

For the input '1+2+3':
Again, before starting convert2 you had alread read '1' and '+', then you read '2' and sum it, then you read '+' and quit the loop. Since '+' is not a numeric digit, you print 'error' and complain.

I think you need to throw away convert2 entirely and rethink your code. No one I know ever does this when I suggest it to them, but good programmers always do: get out a piece of paper and a pencil and draw yourself your problem. Solve it on paper. Once you know how to solve it yourself then it is much easier to tell the computer how to do it.

So, lets think about your problem quickly:

I have a line that looks like this:
1+2+3
How do I solve it myself?
I look at the first number: [B]1[/B]+2+3 I remember that as my current total.

I look at the next thing: 1[B]+[/B]2+3 OK, I need to add something to my current total.

Next: 1+[B]2[/B]+3 The something to add is the number 2. My current total becomes 1+2=3.

Next: 1+2[B]+[/B]3 Addition again.

Next: 1+2+[B]3[/B] The number to add is 3. My new total is 3+3=6.

Next:
No more. All done. The current total is 6.

Now, consider what the computer will need to remember to do that. (It needs to remember the same things you needed to remember):

I have a file I'm reading: infile: text; I read characters. So I need a char variable: c: char; I have a number I just read. So I need an integer: number_just_read: integer; I have a current total. I need another integer: current_total: integer; I also have the type of operation to perform. I can store that as a char: op: char;

Finally, lets consider how I did it:

1
Well, I looked at the first thing. It was a number. In fact, the first thing should always be a number. I could use a procedure just to read characters until I have a complete number. (You had a similar idea when you wrote multidigits. However, you should use a function that uses a loop and returns the number as an integer.)

2
Then I looked at the next thing. It should be an operator. I made a mental note of what kind of thing I should do with the next number I read and the number I already have.

3
I read the next thing. It should be a number again. So I'll just use my function that reads numbers to get it.

4. Now I have two numbers and an operation. I do the operation.

Hmm, let me think. I do some stuff over and over again... Where do I repeat myself? Steps 2, 3, and 4 are what happen over and over again. I quit when there is nothing more to read. I suppose I can notice this in step 4 or step 2. But if I notice this in step 3 there is a problem, since every operator should be followed by a number.

These mental exercises aren't to make you feel stupid or silly. It is how programmers do their programming. Do it yourself (on paper) first. Think about how you did it and make notes about what you needed to do it. Write the steps you used to do it down. Only then, tell the computer to do the same thing.

Hope this helps.

[pocku]

Thank you very much for taking the time to analyze my code! :) I found the paper method very useful and have revised my code. But now, I'm having a problem with my while-loop under convert...

By the way, my prof wants us to code the program using only procedures and parameters so I can't use functions...:

procedure convert ;

begin
   digit1:=ord(c)-ord('0');
   read(infile,c);

   legalinteger:= (ord(c)<=ord('9')) and (ord(c)>=ord('0'));

   while legalinteger and not eoln(infile) do
   begin
      digit1:= digit1*10;
      digit3:= ord(c)-ord('0');
      digit1:= digit1 + digit3;
      read(c);
   end;

   num_just_read:=true;

end; 

The program compiles and runs but it freezes when I enter something like 10+2.

Regarding

writeln('There can''t be an operation sign at the end.');

The program actually compiles if I use " instead of '. It actually doesn't compile if I use ' instead.

[pocku]

I can't seem to find the edit button... but nevermind about what I said about ". I see what you mean, but strangely, the computer lets me compile the program.

[Duoas]

>The program actually compiles if I use " instead of '. It actually doesn't compile if I use ' instead.
Sorry to address this first, but whaaat???? What compiler are you using? If it doesn't take ' but does take " then there is something seriously wrong. ' is Pascal. " is not. Never has been. Never will be.

OK, on to your real problem. :-/

Your loop is predicated upon something that never changes in the loop: legalinteger. The order you do things is also a little disorganized. Presumably you know that c is a digit when you first call convert. However, once you start the procedure, you read another c from the file and assume that it also is a digit. Don't. All your reads should be inside the loop.
Your loop should also be carefully structured so that you do not multiply digit1 by ten unless you already know that the next c is a digit.

Also, I think your professor is a crazy loon. So that's why you are using so many global variables and non-separable procedures. Would he concede to let you pass by reference? procedure fooey( var i: integer ); What you are doing is learning to program badly. There is no better way to say it. If this is really what your professor wants I think he should be fired.

[EDIT] Caught your last post. The edit button disappears once you log out.

[pocku]

Sorry for such a late reply!

What I meant last week about the compiler only letting me use " was that when I have a sentence containg a word like can't, the compiler will only let me compile if I wrap the entire sentence it's in with " rather than ' such that

writeln("There can't be an operation sign at the end.");

Sorry if there was any misunderstanding!

And yes, my prof allows us to pass by reference in this assignment. I have a question though: what difference is there between using global variables and passing by reference? I tried using reference variables but I don't really see any difference.

I also have another question about loops. If I had a read outside the loop and say it read a 'y,' why didn't it just skip my

while legalinteger and not eoln(infile) do

loop and continue on with the rest of program? Isn't y not a legalinteger?

By the way, I did what you said and got the loop fixed up. The program seems to be working fine now. :)

Also, what is considered as learning to program badly and learning to program properly? Maybe it's because I'm a newbie programmer but I don't really understand...

[Duoas]

> Sorry for such a late reply!
Heh, don't worry about it. Sometimes I'll mull things over for a day or two before replying to a thread, just to make sure I give the most intelligent answer I can. When I just automatically hit the "reply" button is when I tend to get into trouble...

I understand about the " vs '. Your compiler is weird, as that's illegal Pascal. The proper way is to double the ' character.

There's a technical term for this but I can never remember it: global variables aren't evil. But they should be avoided. The reason has to do with "state complexity". Computers always do exactly what they are told. We humans have trouble remembering more than a few things at a time, though, so when we have a lot of global variables accessed by a zillion procedures, we can't keep track of what state the program is in at any given time. This is part of the problem you are having with your assignment: there's too much information to remember at once.

The point behind "modular programming" is to reduce the "state" to smaller units that don't depend upon each other. For example, your code currently requires each routine to be aware of which one of digit1, digit2, digit3 it can change at any given time. That's dangerous, because if you mix up the routines the wrong way then you clobber data that you need. You are better off keeping things separate.

For example, any routine that reads from the input can take a reference parameter to the last-read character. What that means is that it only has to worry about one character -the one given to it. If it needs other variables for its personal use, it can do so without worrying about damaging something elsewhere.

function isDigit( c: char ): boolean;
  begin
  result := c in ['0'..'9']
  end;

function readNumber( var f: textFile; var last_c: char ): integer;
  begin
  result := 0;
  while isDigit( last_c ) do begin
    result := (result *10) +ord( last_c ) -ord( '0' );
    read( f, last_c )
    end
  end;

Notice how in this code the last_c reference is always the last character read from the file. If you call the function and the last-read-character is not a digit, then things still work properly: the file hasn't been read any more, the last-read-char hasn't been changed, and you get zero as a result (since no number was read).

The other thing to notice is that the code doesn't rely upon anything other than what was given to it. It doesn't need global variables. It doesn't care what the current state of the program is. All it cares about is the last-read-character and the file --which is all it should care about.

Now, when you use such a procedure, you don't have to care about anything else either! That's great! It frees your mind to worry about the big picture instead of nitty details.

// continued from above
...
// These variables are technically global, but they are limited
// to use in the main block, so they are OK. (They are treated
// as local to the main program block.)
var c: char;
begin
  writeln( 'Type lots of stuff, including numbers. Type ^Z, ENTER to quit.' );

  read( c );
  while not eof do
    begin
    if isDigit( c )
      then writeln( 'You typed the number ', readNumber( input, c ), '.' )
      else

    if c in ['A'..'Z']
      then writeln( 'You typed a capital letter!' );

    read( c )
    end;

  writeln( 'Good-bye.' )
end.

This is what I mean by good vs bad programming. All programming is about how you think. The clarity of your own thought processes manifest themselves in the programs you write. It doesn't matter if you use Pascal or C++ or Java or Scheme or whatever. What matters is how you think about it. Learning to program is, in the most correct sense, learning to think.

Now, don't feel to bad. You are just beginning, but you now have a knowledge that will help you learn to program well: by learning to examine your own thought processes you learn to program. So, if something seems big, or unmanageable, think about how to break it down into smaller tasks. Once done, concentrate on one task at a time. Smaller tasks may themselves be broken into even smaller tasks, and so on. (This is called the "top-down" approach, BTW.) Then, combine your smaller tasks together and build back up to a completed program.

I hope this makes sense. If I have confused you about any particular thing, ask for more.

[EDIT] I can't answer your question about loops without more info. Did you read y and forget to set legalinteger before the while statement? Things only work in order... Frankly, I'd use isDigit instead of legalinteger, since isDigit always gives the correct answer, anytime, where legalinteger might not be right if it isn't set right before you need it...

Hope this helps.