PDA

View Full Version : GAH!!!! Too much tinkering and I've broke it. Help!



stevengreen22
18-04-2011, 12:48 PM
Hi guys,

I've screwed up what was a reasonable attempt at a program. Too much tinkering while at work, not saving properly and now I've screwed it and can't see the obvious errors.

Help!!! :) Please.

As a bit of background, Its a date validation program, supposed to be simple.
you write in 02 02 2012, it will tell you its the 2nd Feb 2012 with suitable error handling for days and months that don't exist. I will encounter an issue with maxdays for months soon but for the time being I cannot get the month to display!



program EX10DATEVALIDATIONSG(input,output);
uses crt;
const Title:string = ' EX10 DATE VALIDATION ';
By:string = ' By Steve Green ';

var dayNo,monthNo,yearNo:integer; //the intitial input by the user

monthnumber,maxdays:integer;

month,day:string;

procedure menu;
begin
ClrScr;
textcolor(red);
writeln(Title);
writeln;
writeln(by);
writeln;
textcolor(green);
writeln;
writeln('Hello, My name is Menu.');
writeln('I am here to make things easier');
writeln('To exit the program press 0');
writeln; //seperate procedure?

end;
procedure cont;
begin
writeln;
writeln;
writeln;
textcolor(red);
writeln ('Press <Return> to continue');
readln ();
end;

procedure monthcheck;

begin case monthnumber of //use an array instead? {Naming of Months}
1: month :=('January ');
2: month :=('Febuary ');
3: month :=('March ');
4: month :=('April ');
5: month :=('May ');
6: month :=('June ');
7: month :=('July ');
8: month :=('August ');
9: month :=('September ');
10:month :=('October ');
11:month :=('November ');
12:month :=('December ');
end;

case monthnumber of {This section gives the}
4,6,9,11: maxdays := 30; {months their correct lengths}
1,3,5,7,8,10,12: maxdays := 31;
2: if yearNo mod 4=0 then maxdays :=29 else maxdays := 28;
end;
end;
procedure daycheck;
begin
case dayNo of
1,21,31: write (dayNo,'st'); // sets prefix for day
2,22: write (dayNo,'nd');
3,23: write (dayNo,'rd');
4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,24,
25,26,27,28,29,30: write(dayNo,'th');
end;
end;


begin {main program}

menu;

writeln;
write('Please enter the date to continue, enter in this format, 00 00 0000 and press <enter> to continue.');
writeln;
readln(dayNo,monthNo,yearNo);



if (dayNo<=0) or (dayNo >31) then
begin
writeln('Sorry, There are only no of days in this month!'); //error handling
writeln('Please enter another day');
read(dayNo);
end;
if (dayNo>=1) or (dayNo<=31) then //should have month / day count check first?
//if day no is satifies run proc daycheck (for prefix)
daycheck;


if (monthNo <=0) or (monthNo >12) then
begin //error handling for months
writeln('Sorry, There are only 12 months in a year!');
writeln('Please enter the month again');
read(monthNo);
end;
if (monthNo > 0) and (monthNo <= 12) then

monthcheck;

write(month); // *^&?ì$(*^&?ì(*&(GAH!!!!!!!!!!!!!!!!!


write(' ',yearNo);


writeln;
writeln


// cont;

end.


I did have a working version that would display date etc using an array of month names and that was working, I then started changing things and lost the original save. Maybe I'm going about this wrong by using case instead of arrays? Any input / help would be hugely appreciated.

Thanks

stevengreen22
18-04-2011, 02:34 PM
had a little play and managed to get part of the original working again.



program EX10DATEVALIDATIONSG(input,output);
uses crt;
const Title:string = ' EX10 DATE VALIDATION ';
By:string = ' By Steve Green ';

var dayNo,monthNo,yearNo:integer;
maxdays,monthmax,monthnumber:integer;

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

procedure menu;
begin
ClrScr;
textcolor(red);
writeln(Title);
writeln;
writeln(by);
writeln;
textcolor(green);
writeln;
writeln('Hello, My name is Menu.');
writeln('I am here to make things easier');
writeln('To exit the program press 0');
writeln; //seperate procedure?
writeln;

end;
procedure cont;
begin
writeln;
writeln;
writeln;
textcolor(red);
writeln ('Press <Return> to continue');
readln ();
end;


procedure monthcheck;

begin
case monthmax of {This section gives the}
4,6,9,11: maxdays := 30; {months their correct lengths}
1,3,5,7,8,10,12: maxdays := 31;
2: if yearNo mod 4=0 then maxdays :=29 else maxdays := 28;
end;
end;


procedure daycheck;
begin
case dayNo of
1,21,31: write (dayNo,'st'); // sets prefix for day
2,22: write (dayNo,'nd');
3,23: write (dayNo,'rd');
4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,24,
25,26,27,28,29,30: write(dayNo,'th');
end;
end;


begin {main program}

menu;

write('Please enter the date to continue, enter in this format, 00 00 0000.');
writeln;
readln(dayNo,monthNo,yearNo);

if (dayNo<=0) or (dayNo>31) then
begin
writeln('Sorry, There are only no of days in this month!'); //error handling
writeln('Please enter another day');
readln(dayNo);
end;
if (dayNo>=1) or (dayNo<=31) then

daycheck;


if (monthNo <=0) or (monthNo >12) then
begin
monthcheck;

writeln('Sorry, There are only 12 months in a year!');
writeln('Please enter the month again');
read(monthNo);
end;
if (monthNo > 0) and (monthNo <= 12) then

monthcheck;

write(' ',monthnames[monthNo]);


write(' ',yearNo);


writeln;
writeln;


cont;

end.



Can someone now please help with getting the maxday part to work, I'm still not convinced I should be using a case statement.

thanks.

User137
18-04-2011, 06:51 PM
1) Please fix indent of code. That is very hard to read.

2) Why are you calling monthcheck even if the month is entered wrong?

if (monthNo <=0) or (monthNo >12) then
begin
monthcheck;

3) Public variables and procedures are bad coding style. You should learn to use functions.

For example monthcheck could aswell be written as:

function getMaxDays(month, year: integer): integer;
// Monthcheck as a name doesn't tell much. You can use the names for understandability
begin
case month of {This section gives the}
4,6,9,11: result := 30; {months their correct lengths}
2: if year mod 4=0 then result := 29
else result := 28;
else result := 31;
end;
end;

4) You had monthMax instead of monthNo in the case inside monthcheck procedure.

edit: 5) What are monthmax, monthnumber variables anyway? They aren't used or initialized anywhere.

stevengreen22
18-04-2011, 08:27 PM
Hi,

Thanks for having a lok at it and finding the errors, it was a fking mess.

Those variables were all mixed up, i wrote one, then rewrote at work as i didn't have the original files then today i sort of managed to merge them both and destroy them at the same time while losing both files :) clever huh.

The variables...should be dayNo, MonthNo and YearNo(the user inputs)

I then have maxday (max no of days per month) the others were left over from the other and have been removed. I may need another depending on the leap year, whether that is used as a seperate proc or not.

I've not touched on functions yet and to be honest...don't really have a clue how they work, will be reading up on them tomorrow.
It looks like it takes global variables / procedure and makes it one instead of many..?

It shouldn't have been doing the check if month was incorrect, my mistake.

I also havent got maxday to work properly yet. So far the best I have is where it will ask for a correct day and/or month before giving output. I think I need to put a loop/repeat around the error handling bit in case someone types in 13 twice as a month.

Sorry about the indentation. -have amaended some.


program EX10DATEVALIDATIONSG(input,output);
uses crt;
const Title:string = ' EX10 DATE VALIDATION ';
By:string = ' By Steve Green ';

var dayNo,monthNo,yearNo:integer;
maxdays:integer;

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

procedure menu;
begin
ClrScr;
textcolor(red);
writeln(Title);
writeln;
writeln(by);
writeln;
textcolor(green);
writeln;
writeln('Hello, My name is Menu.');
writeln('I am here to make things easier');
writeln('To exit the program press 0');
writeln; //seperate procedure?
writeln;

end;
procedure cont;
begin
writeln;
writeln;
writeln;
textcolor(red);
writeln ('Press <Return> to continue');
readln ();
end;


procedure monthcheck;

begin
case monthNo of {This section gives the}
4,6,9,11: maxdays := 30; {months their correct lengths}
1,3,5,7,8,10,12: maxdays := 31;
2: if yearNo mod 4=0 then maxdays :=29
else maxdays := 28;
end;
end;


procedure daycheck;
begin
case dayNo of
1,21,31: write (dayNo,'st'); // sets prefix for day
2,22: write (dayNo,'nd');
3,23: write (dayNo,'rd');
4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,24,
25,26,27,28,29,30: write(dayNo,'th');
end;
end;


begin {main program}

menu;
write('Please enter the date to continue, enter in this format, 00 00 0000.');
writeln;
readln(dayNo,monthNo,yearNo);

if (dayNo<=0) or (dayNo>31) then
begin
writeln('Sorry, There are only no of days in this month!'); //error handling
writeln('Please enter another day');
readln(dayNo);
end;
if (dayNo>=1) or (dayNo<=31) then
daycheck;


if (monthNo <=0) or (monthNo >12) then
begin
writeln('Sorry, There are only 12 months in a year!');
writeln('Please enter the month again');
read(monthNo);
end;
if (monthNo > 0) and (monthNo <= 12) then
monthcheck;
write(' ',monthnames[monthNo]);

write(' ',yearNo);

writeln;
writeln;
cont;

end.

stevengreen22
20-04-2011, 02:40 PM
Ello,

I've made the code a bit better (I think) - It now works as the program shoudl. am missing a couple of repeats for the error handling part of the program (at the moment it will only ask for the correct date once)
I also have no functions. I am still reading and testing implementation.
For the time being, woudl you mind havign a look and picking any obvious holes in it?
(for the year, it will be limited to within a set range)


program EX10DATEVALIDATIONSG(input,output);
uses crt;
const Title:string = ' EX10 DATE VALIDATION ';
By:string = ' By Steve Green ';

var dayNo,monthNo,yearNo:integer; //global
maxdayNo:integer; //within case

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

procedure menu;
begin
ClrScr;
textcolor(red);
writeln(Title);
writeln;
writeln(by);
writeln;
textcolor(green);
writeln;
writeln('Hello, My name is Menu.');
writeln('I am here to make things easier');
writeln('To exit the program press 0');
writeln; //seperate procedure?
writeln;
end;

procedure cont;
begin
writeln;
writeln;
writeln;
textcolor(red);
writeln ('Press <Return> to continue');
readln ();
end;

procedure monthcheck;
begin
case monthNo of {This section gives the}
4,6,9,11: maxdayNo := 30; {months their correct lengths}
1,3,5,7,8,10,12: maxdayNo := 31;
2: if yearNo mod 4=0 then maxdayNo :=29
else maxdayNo := 28; //leap yr
end;
end;

procedure daycheck;
begin
case dayNo of
1,21,31: write (dayNo,'st'); // sets prefix for day
2,22: write (dayNo,'nd');
3,23: write (dayNo,'rd');
4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,24,
25,26,27,28,29,30: write (dayNo,'th');
end;
end;

begin {main program}

menu; //can maybe have the input as menu / sep proc?
write('Please enter the date to continue, enter in this format, DD MM YY.');
writeln;
readln(dayNo,monthNo,yearNo);
//END OF USER INPUT

//BEGIN ERROR HANDLING FOR DAYS WITHIN A MONTH
monthcheck;
if (dayNo> maxdayNo) or (dayNo <=0) then
begin
writeln('There are ',maxdayNo, ' of days in this month!');
writeln('Please select a valid day');
read(dayNo);
end;
if (dayNo>=1) or (dayNo<=31) then
daycheck;

//ERROR HANDLING FOR MONTHS
if (monthNo <=0) or (monthNo >12) then
begin
writeln('Sorry, There are only 12 months in a year!');
writeln('Please enter the month again');
read(monthNo);
end;
if (monthNo > 0) and (monthNo <= 12) then
monthcheck;
write(' ',monthnames[monthNo]);

//NEED ERROR HANDLING FOR YEARS (OUTSIDE OF RANGE ETC)
if (yearNo <=99) and(yearNo >12) then
write(' 19',yearNo);
if (yearNo <=09) and (yearNo >=00) then
write(' 200',yearNo);
if (yearNo =10) and (yearNo <=12) then
write(' 20',yearNo);

writeln;
writeln;
cont;
end.

Thanks

chronozphere
20-04-2011, 02:43 PM
4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,24,
25,26,27,28,29,30

Why do you do this?

Please do it like this:


4..30


Also your code indentation is very counter-intuitive:


if (yearNo <=99) and(yearNo >12) then
write(' 19',yearNo);
if (yearNo <=09) and (yearNo >=00) then
write(' 200',yearNo);
if (yearNo =10) and (yearNo <=12) then
write(' 20',yearNo);

Should be:


if (yearNo <=99) and(yearNo >12) then
write(' 19',yearNo);
if (yearNo <=09) and (yearNo >=00) then
write(' 200',yearNo);
if (yearNo =10) and (yearNo <=12) then
write(' 20',yearNo);


I think that the format() function will also help you to print your date more easily. The sollution with tons of If statements is just messy IMHO.

;)

stevengreen22
20-04-2011, 03:14 PM
Hi mate,

thanks for that, I missed the 4..30 bit - :)

I'd liek to use functions as not used them before but jsut gettign to grips with them, the indent was ugly. I still havent found my'way' of doing it yet.

was there anything else you noticed?

Also, I think I might have to change the error handling. if you type in 36 36 36 (it will first say there are 0 days in this month and then carry out checking the numbers after. the logic would be better to check the month first i think.

stevengreen22
20-04-2011, 04:37 PM
GAH!!! the 4..30: doesn't work - I get an error message?!?!?

User137
20-04-2011, 08:05 PM
Because you have another case with same range included (The error message propably tells that). You should use "else" in this case, or split 4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,24,25 ,26,27,28,29,30 in smaller parts like 4..20, 24..30

stevengreen22
21-04-2011, 06:35 PM
of course, i was having a 'special' moment. wll work on it some more later to try and improve it.