Pascal app. feedback

Dev Shed Forums Sponsor:

Stop making mediocre tutorials.The best tutorials are video! Camtasia Studio makes it easy to create engaging, buzz-building screen videos at any size, in any popular format. Download the free trial!

April 26th, 2006, 03:42 PM

lingon

C++arl!

Join Date: Mar 2006

Location: Stockholm

Posts: 165

Time spent in forums: 1 Day 5 h 59 m 35 sec
Reputation Power: 12

Pascal app. feedback

I started programming two weeks ago, and begun with pascal since I've heard that it is a good language to start with. Anyway I made this encryption program and would like some feedback on it. Like are there better ways of doing this and such. Any ideas are appreciated!
/lingon

Attached Files

Encryptor.zip (38.5 KB, 171 views)

April 30th, 2006, 04:41 AM

Schol-R-LEA

Commie Mutant Traitor

Join Date: Jun 2004

Location: The People's Republic of Berkeley

Posts: 1,038

Time spent in forums: 3 Weeks 3 Days 8 h 32 m 33 sec
Reputation Power: 330

To begin with, you generally don't want to post code as an attachment unless it is actually too large to post in the text of a message; more importantly, you should not post an attachment containing an executable, period. First off, the executable is of little if any help, as you really can't debug it; unless you are having problems with the executable itself, the important material is the source. Second, there have been more than a few jerks who have tried to get people to run viruses or backdoors that way, and most of the regulars, upon seeing a zipped file, will just ignore the discussion thread rather than deal with that. Finally, the zipped executable takes up a lot more space than the source, and you have to go out of your way to download it; even aside from the virus problem, most readers simply would want to hassle with it.

Second, you generally want to indent you code to match the nesting of the expressions, like so:

Pascal Code:

Original - Pascal Code

program Encryptor;              {Krypterar och avkrypterar meddelanden.}
uses
    crt;
var
   s : string;
   val, tecken : char;
   a, tal, l : byte;
   x : integer;
   f, fi, fis : text;

procedure encode;
   begin
        clrscr;
        assign(f, 'C:SDATA.DAT');
        assign(fi, 'C:IDATA.DAT');
        rewrite(fi);
        rewrite(f);
        writeln('Enter your message below.');
        writeln('-------------------------');
        readln(s);
        l := length(s);
        tal := 0;
        writeln();
        writeln('This is the encoded message.');
        writeln('----------------------------');
        for a := 1 to l do
        begin
            inc(tal);
            tecken := s[tal];
            write(ord(tecken),' ');
            write(f,ord(tecken),' ');  {H„r skrivs den som string.}
            write(fi,x,ord(tecken),' '); {H„r skrivs den som integer.}
        end;
        close(f);
        close(fi);
        writeln();
        writeln();
        writeln('The encoded message has been saved.');
        writeln();
        writeln('Press ENTER to return to main menu.');
        readln();
   end;

{Man m†ste ha tv† text-filer, en f”r att visa upp som string och}
   {en, fi, som „r en integer eftersom chr(x) kr„ver att x = integer.}

procedure decode;
   begin
        clrscr;
        writeln('Enter the numbers manually or read data from file.');
        writeln('--------------------------------------------------');
        writeln('1:Read from file.');
        readln(val);
        if val = '1' then
        begin
           assign(f, 'C:SDATA.DAT');
           assign(fi, 'C:IDATA.DAT');
           assign(fis, 'C:Message.txt');
           rewrite(fis);
           reset(fi);
           reset(f);
           read(f,s);
           writeln();
           writeln('This is the encoded message.');
           writeln('----------------------------');
           write(s);
           l := length(s);
           writeln();{vet inte varf”r jag m†ste ha tv† tomma :S}
           writeln();
           writeln('This is the decoded message.');
           writeln('----------------------------');
           for a := 1 to l div 2 do
           begin
               inc(tal);
               read(fi,x);
               write(chr(x));
               write(fis,chr(x));
           end;
           close(f);
           close(fi);
           close(fis);
           writeln();
           writeln();
           writeln('The decoded message has been saved.');
           writeln();
           writeln('Press ENTER to return to main menu.');
           readln();
        end;
   end;

procedure info;
   begin
        clrscr;
        writeln('Info');
        writeln('----');
        writeln('Made by lingon');
        writeln();
        writeln('The purpose of this program is to make it easy for you to encrypt messages.');
        writeln();
        writeln('It encrypts by changing the letters into thier respective value using the ANCSIIchart.');
        writeln();
        writeln('I recommend keeping your messages small since strings canït hold more than 255  values.');
        writeln();
        writeln('To encode a message, simply enter the text and hit enter.');
        writeln('Two .DAT files will automatically be created in the folder that the program runsfrom. These contain the encoded message and can be sent to anyone and opend and decoded using this program.');
        writeln();
        writeln('To decode a message, simply enter the numbers in the message. Be sure to enter  them as they are, in pairs or threes, otherwise the computer wont know wich number belongs to wich.');
        Writeln('You can also open the .DAT files made when encoding a message, the program will then decode the data stored within and write it down in a .TXT file.');
        writeln();
        writeln('Press ENTER to return to main menu.');
        readln(val);
   end;

procedure password;
   begin
        clrscr;
        write('Enter password:');
        readln(s);
        if s = '1798' then
        begin
           writeln('Acses graanted. Welcome.');
           delay(1500);
           exit;
        end
        else
        begin
             writeln('Acses denied. Shuting down.');
             delay(1500);
             halt;
        end;
   end;

begin
     password;
     repeat
           clrscr;
           Writeln('*****Encryptor by lingon*****');
           Writeln('-----------------------------');
           writeln('1:Encode Message');
           writeln('2:Decode Message');
           writeln('3:Info');
           writeln('4:Exit');
           readln(val);
           if val = '1' then
              Encode;
           if val = '2' then
              Decode;
           if val = '3' then
              info;
           if val = '4' then
              halt
           else
              writeln('Invalid input.');
     until (val='3');
end.

program Encryptor;              {Krypterar och avkrypterar meddelanden.}
uses
    crt;
var
   s : string;
   val, tecken : char;
   a, tal, l : byte;
   x : integer;
   f, fi, fis : text;
 
   procedure encode;
   begin
        clrscr;
        assign(f, 'C:SDATA.DAT');
        assign(fi, 'C:IDATA.DAT');
        rewrite(fi);
        rewrite(f);
        writeln('Enter your message below.');
        writeln('-------------------------');
        readln(s);
        l := length(s);
        tal := 0;
        writeln();
        writeln('This is the encoded message.');
        writeln('----------------------------');
        for a := 1 to l do
        begin
            inc(tal);
            tecken := s[tal];
            write(ord(tecken),' ');
            write(f,ord(tecken),' ');  {H„r skrivs den som string.}
            write(fi,x,ord(tecken),' '); {H„r skrivs den som integer.}
        end;
        close(f);
        close(fi);
        writeln();
        writeln();
        writeln('The encoded message has been saved.');
        writeln();
        writeln('Press ENTER to return to main menu.');
        readln();
   end;
 
   {Man m†ste ha tv† text-filer, en f”r att visa upp som string och}
   {en, fi, som „r en integer eftersom chr(x) kr„ver att x = integer.}
 
   procedure decode;
   begin
        clrscr;
        writeln('Enter the numbers manually or read data from file.');
        writeln('--------------------------------------------------');
        writeln('1:Read from file.');
        readln(val);
        if val = '1' then
        begin
           assign(f, 'C:SDATA.DAT');
           assign(fi, 'C:IDATA.DAT');
           assign(fis, 'C:Message.txt');
           rewrite(fis);
           reset(fi);
           reset(f);
           read(f,s);
           writeln();
           writeln('This is the encoded message.');
           writeln('----------------------------');
           write(s);
           l := length(s);
           writeln();{vet inte varf”r jag m†ste ha tv† tomma :S}
           writeln();
           writeln('This is the decoded message.');
           writeln('----------------------------');
           for a := 1 to l div 2 do
           begin
               inc(tal);
               read(fi,x);
               write(chr(x));
               write(fis,chr(x));
           end;
           close(f);
           close(fi);
           close(fis);
           writeln();
           writeln();
           writeln('The decoded message has been saved.');
           writeln();
           writeln('Press ENTER to return to main menu.');
           readln();
        end;
   end;
 
   procedure info;
   begin
        clrscr;
        writeln('Info');
        writeln('----');
        writeln('Made by lingon');
        writeln();
        writeln('The purpose of this program is to make it easy for you to encrypt messages.');
        writeln();
        writeln('It encrypts by changing the letters into thier respective value using the ANCSIIchart.');
        writeln();
        writeln('I recommend keeping your messages small since strings canït hold more than 255  values.');
        writeln();
        writeln('To encode a message, simply enter the text and hit enter.');
        writeln('Two .DAT files will automatically be created in the folder that the program runsfrom. These contain the encoded message and can be sent to anyone and opend and decoded using this program.');
        writeln();
        writeln('To decode a message, simply enter the numbers in the message. Be sure to enter  them as they are, in pairs or threes, otherwise the computer wont know wich number belongs to wich.');
        Writeln('You can also open the .DAT files made when encoding a message, the program will then decode the data stored within and write it down in a .TXT file.');
        writeln();
        writeln('Press ENTER to return to main menu.');
        readln(val);
   end;
 
   procedure password;
   begin
        clrscr;
        write('Enter password:');
        readln(s);
        if s = '1798' then
        begin
           writeln('Acses graanted. Welcome.');
           delay(1500);
           exit;
        end
        else
        begin
             writeln('Acses denied. Shuting down.');
             delay(1500);
             halt;
        end;
   end;
 
 
begin
     password;
     repeat
           clrscr;
           Writeln('*****Encryptor by lingon*****');
           Writeln('-----------------------------');
           writeln('1:Encode Message');
           writeln('2:Decode Message');
           writeln('3:Info');
           writeln('4:Exit');
           readln(val);
           if val = '1' then
              Encode;
           if val = '2' then
              Decode;
           if val = '3' then
              info;
           if val = '4' then
              halt
           else
              writeln('Invalid input.');
     until (val='3');
end.

While it may seem a bit odd at first, you'll find that it makes a tremendous difference in the readability of the code once you get used to it. If you use an editor or IDE that recognizes Pascal code (e.g., Emacs, Scite, vim, Zeus Edit, EditPad Pro, Delphi, Dev-Pascal, Dr Pascal, etc.), it should indent the code for you, or even allow you to indent a section of existing code automatically.

Third, for the menu part in the main program block, since val is a simple scalar variable whose value you are testing against a specific list of possible values, you might want to use a case statement instead of the successive ifs:

Pascal Code:

Original - Pascal Code
case val of '1': Encode; '2': Decode; '3': info; '4': halt; default: writeln('Invalid input.'); end;

           case val of
               '1':
                  Encode;
               '2':
                  Decode;
               '3':
                  info;
               '4':
                  halt;
               default:
                  writeln('Invalid input.');
           end;

This might make it clearer and easier to change later.

Fourth, you are using global variables for everything, rather than local ones; while it is not wrong per se, this is usually considered a bad practice. You should try to have the variable you are using visible in only the parts of the program that use them. You can also use arguments to further reduce the need for globals, by allowing you to pass values to a procedure or function without making them visible elsewhere. Using the scoping rules for nested procedures can also let you share variables between procedures without making them global in scope.

(On a side note: using lowercase letter 'l' as a variable name is a Bad Thing, since in some fonts it can too easily be mistaken for either the capitalized 'I' or the numeral '1'; the latter in particular has considerable potential for confusion.)

Lastly (for now), you might want to decompose the program into smaller pieces; specifically, it is a good idea to separate the user interface code from the actual computation code. In Pascal, you specifically can nest procedures and functions inside of each other, which makes it easy to break down a single procedure into many sub-components. You could, for example, do something like:

Pascal Code:

Original - Pascal Code
procedure password; var s: string; function confirmed(s : string): boolean; begin confirm := (s = '1798'); end; begin clrscr; write('Enter password:'); readln(s); if confirmed(s) then begin writeln('Access granted. Welcome.'); delay(1500); exit; end else begin writeln('Access denied. Shutting down.'); delay(1500); halt; end; end;

   procedure password;
        var 
           s: string;
 
        function confirmed(s : string): boolean;
        begin
            confirm := (s = '1798');
        end;
 
    begin
        clrscr;
        write('Enter password:');
        readln(s);
        if confirmed(s) then
        begin
            writeln('Access granted. Welcome.');
            delay(1500);
            exit;
        end
        else
        begin
            writeln('Access denied. Shutting down.');
            delay(1500);
            halt;
        end;
   end;

This particular example is trivial, but with some thought you'll see how this can be applied more generally.

I'll try to take a closer look at things later.

Comments on this post

Yegg` agrees!

__________________
Rev First Speaker Schol-R-LEA;2 JAM LCF ELF KoR KCO BiWM TGIF
#define KINSEY (rand() % 7) λ Scheme is the Red Pill
Scheme in Short • Understanding the C/C++ Preprocessor
Taming Python • A Highly Opinionated Review of Programming Languages for the Novice, v1.1

FOR SALE: One ShapeSystem 2300 CMD, extensively modified for human use. Includes s/w for anthro, transgender, sex-appeal enhance, & Gillian Anderson and Jason D. Poit clone forms. Some wear. $4500 obo. tverres@et.ins.gov

Last edited by Schol-R-LEA : April 30th, 2006 at 10:39 AM.

April 30th, 2006, 10:35 AM

lingon

C++arl!

Join Date: Mar 2006

Location: Stockholm

Posts: 165

Time spent in forums: 1 Day 5 h 59 m 35 sec
Reputation Power: 12

Okay, thanks alot dude. I will definetly follow those tips.