Helpful Information
 
 
Category: Other Programming Languages
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

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:

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 (http://www.gnu.org/software/emacs/), Scite (http://www.scintilla.org/SciTE.html), vim (http://www.vim.org/), Zeus Edit (http://www.zeusedit.com/pascal.html), EditPad Pro (http://www.editpadpro.com/editdelphi.html), Delphi (http://www.borland.com/us/products/delphi/index.html), Dev-Pascal (http://www.bloodshed.net/devpascal.html), Dr Pascal (http://www.visible-software.com/prod-dp.html), 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:
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:
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.

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

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 (http://www.gnu.org/software/emacs/), Scite (http://www.scintilla.org/SciTE.html), vim (http://www.vim.org/), Zeus Edit (http://www.zeusedit.com/pascal.html), EditPad Pro (http://www.editpadpro.com/editdelphi.html), Delphi (http://www.borland.com/us/products/delphi/index.html), Dev-Pascal (http://www.bloodshed.net/devpascal.html), Dr Pascal (http://www.visible-software.com/prod-dp.html), 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.

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.

According to the FreePascal documentation (http://www.freepascal.org/docs.html), 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.

According to the FreePascal documentation (http://www.freepascal.org/docs.html), 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 :rolleyes:










privacy (GDPR)