PDA

View Full Version : Please check code...



stevengreen22
19-03-2011, 04:57 PM
Hi guys,

I've this assignment to create a menu for a pub based on the season. The tutor has given a small part of the code, but within my program the if - else statements become redundant.

I'm thinking it's unlikley for him to give us code that doesn't mean anything in the long run.

Would you mind having a look and seeing if there's an alternative way for this program to work that you think is more efficient. I'm getting a bit anal and wanting the perfect program now :) Have only dropped 4 marks out of a possible 600 so far so looking good....

his code:


summer:= (month > 5) and (month <=8);
if summer then
writeln('Melon')
else writeln('Oysters');
write('Roast Chicken with ');
if summer then
writeln('Green Salad')
else writeln('Two Veg');


My code - You can see where I've commented out the if-else (I also REALLy like using procedures.


program EX8PUBMENU(input,output);
uses crt;
const Title:string = ' EX8 ';
By:string = ' By Steve Green ';

var month:integer;
autumn,winter,spring,summer:boolean;



procedure sumr;
begin
summer:= (month > 5) and (month <=8);
//if summer then
writeln('Melon')
// else writeln('Oysters');
write('Roast Chicken with ');
// if summer then
writeln('Green Salad')
// else writeln('Two Veg');
end;

procedure autm;
begin
autumn:=(month>8) and (month<=11);
if autumn then
writeln ('Soup')
else writeln('no soup');
write('Broth with ');
if autumn then
writeln('Bread')
else writeln('Dumplings');
end;

----------THE OTHER 2 SEASONS GO HERE--------------

begin {main program}

begin
repeat
menu; -------IS A PROCEDURE
if month>12 then
begin
writeln('Sorry, Surely you know there are only 12 months in a year?!?!?!?');
cont; ------------THIS IS A PRESS RETURN TO CONT PROCEDURE
end;

case month of------------THIS NEEDS TO BE TIDIED, THE 'WRITELN' MAY BE INC
1:begin IN THE PROCEDURE.
clrscr;
WRITELN('Jan!');
writeln; wint;cont;
end;
2:begin
WRITELN('Feb!');
writeln;wint;cont;
end;
3:begin
clrscr;
WRITELN('Mar!');
writeln; spri;cont;
end;
4:begin
clrscr;
WRITELN('April!');
writeln;
spri;cont;
end;
5:begin
clrscr;
WRITELN('May!');
writeln;
sumr; cont;
end;
6:begin
clrscr;
WRITELN('June!');
writeln; sumr;cont;
end;
7:begin
WRITELN('July!');
writeln;
sumr;cont; end;
8:begin
clrscr;
WRITELN('August!');
writeln;sumr;cont; end;
9:begin
clrscr;
WRITELN('September!');
writeln;
autm;cont;
end;
10:begin
clrscr;
WRITELN('October!');
writeln;
autm; cont; end;
11:begin
clrscr;
WRITELN('November!');
writeln; autm; cont;
end;
12:begin
WRITELN('December!');
writeln; wint;cont;
end;
end;
until month = 0;


end;

//end;
end.


Thanks for taking the time to look. This is fully functional and does what is asked in the question but...I want to make it better, it needs to be cleaned up and so on with colours and spacing but you get the idea.

The worse thing about it that I can see is the case statement is drawn out but I'm new to these and arrays.

code_glitch
19-03-2011, 05:11 PM
Hmm... Perhaps you could step out of the console into the SDL/OpenGl if you want it to be impressive... If its a quick mock-up you can always go for some easy to use graphics library instead of writeln...

PS: You might want to indendt that case statement... Its a bit annoying. What you could do is have a string variable that stores what you write or something like that and have a section that processes it and defines that variable then all you need is a single writeln() to write that variable...

stevengreen22
19-03-2011, 08:52 PM
opengl? Dude, that would be awesome but well outside my scope / knowledge base at this moment....might have a peek though as it would be awesome.

What I realllllly want to know is why he would give us code that is virtually redundant.

the case statement will be indented:)

with the string var, I don't know how to 'temp' store what i write and I don't think thats wanted on this one. Although...how would i do that? say you wanted to check 10 numbers and find which of them were +/- or zero by using boolean, and then to display the result...how woudl that be done? sorry for going off on one, its great to be able to ask people who know a great deal more than me.

User137
19-03-2011, 09:44 PM
Tip on using array:

var
monthNames: array[1..12] of string = ('January','February','March','April','May','June' ,'July','August','September','October','November', 'December');

... In the main program where you go through months in a loop:

repeat
menu;
...
clrscr;
if (month > 0) and (month <= 12) then
writeln(monthNames[month],'!'+#13); // #13 is a line change character
case month of
12,1,2: wint;
3,4: spri;
5..8: sumr;
9..11: autm;
end;
until month = 0;

Also in this code etc:

procedure autm;
begin
autumn:=(month>8) and (month<=11);
You don't need to check if it is autumn, it will always be true because you only call this procedure it in that case.
You can delete all these variables (autumn,winter,spring,summer:boolean; )

Still want to fix the indent on teacher's code :p It is very important that the code is understandable. Sometimes wrong indent makes programmer think that part of code belongs inside IF/Else structure when it doesn't.

summer:= (month > 5) and (month <=8);
if summer then
writeln('Melon')
else
writeln('Oysters');
write('Roast Chicken with ');
if summer then
writeln('Green Salad')
else
writeln('Two Veg');