#1
  1. C++arl!
    Devshed Newbie (0 - 499 posts)

    Join Date
    Mar 2006
    Location
    Stockholm
    Posts
    165
    Rep Power
    18

    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
  2. #2
  3. Commie Mutant Traitor
    Devshed Intermediate (1500 - 1999 posts)

    Join Date
    Jun 2004
    Location
    Alpharetta, GA
    Posts
    1,806
    Rep Power
    1570
    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:
    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),' ');  {Hr skrivs den som string.}
                write(fi,x,ord(tecken),' '); {Hr 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 mste ha tv text-filer, en fr att visa upp som string och}
       {en, fi, som r en integer eftersom chr(x) krver 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 varfr jag mste 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 cant 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<img src="http://images.devshed.com/fds/smilies/biggrin.gif" border="0" alt="" title="Big Grin" class="inlineimg" />ecode 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:
               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:
       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
    Last edited by Schol-R-LEA; April 30th, 2006 at 10:39 AM.
    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
  4. #3
  5. C++arl!
    Devshed Newbie (0 - 499 posts)

    Join Date
    Mar 2006
    Location
    Stockholm
    Posts
    165
    Rep Power
    18
    Okay, thanks alot dude. I will definetly follow those tips.
  6. #4
  7. Commie Mutant Traitor
    Devshed Intermediate (1500 - 1999 posts)

    Join Date
    Jun 2004
    Location
    Alpharetta, GA
    Posts
    1,806
    Rep Power
    1570
    No prob. Oh, I don't know if you caught this when I added it above, so here's something I forgot to put in the earlier post originally: 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. Just thought I'd mention it, though I don't know what you're using now.
    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
  8. #5
  9. C++arl!
    Devshed Newbie (0 - 499 posts)

    Join Date
    Mar 2006
    Location
    Stockholm
    Posts
    165
    Rep Power
    18
    I use free pascal right now and it makes such large indent 'steps' if ya know what I mean and thats why I have'nt cared about indenting, but I'll start with it now.
  10. #6
  11. Commie Mutant Traitor
    Devshed Intermediate (1500 - 1999 posts)

    Join Date
    Jun 2004
    Location
    Alpharetta, GA
    Posts
    1,806
    Rep Power
    1570
    According to the FreePascal documentation, section 6.11.7, you can set the tab spacing under Options: Environment...: Editor....

    BTW, I mad an error in the case statement: in most Pascal dialects, the default option for the case is otherwise, not default:. The original standard Pascal didn't actually have a default clause, IIRC, but Object Pascal does.
    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
  12. #7
  13. C++arl!
    Devshed Newbie (0 - 499 posts)

    Join Date
    Mar 2006
    Location
    Stockholm
    Posts
    165
    Rep Power
    18
    Originally Posted by Schol-R-LEA
    According to the FreePascal documentation, section 6.11.7, you can set the tab spacing under Options: Environment...: Editor....

    BTW, I mad an error in the case statement: in most Pascal dialects, the default option for the case is otherwise, not default:. The original standard Pascal didn't actually have a default clause, IIRC, but Object Pascal does.
    Sweet. thnx for helping such a noob

IMN logo majestic logo threadwatch logo seochat tools logo