any good

**Barry Schwarz** · Nov 14 '05, 11:56 AM

Re: any good

On Sat, 21 Aug 2004 13:06:35 +0000 (UTC), Darklight
<nglennglen@net scape.net> wrote:
[color=blue]
>posted by ash4640 These question posted 15/8/04
>How do I write these programs in c I am a beginner learning c.
>1.
>write a program to read 2 inetegers with the following signficance the
>first integer value represents a time of day on a 24hour clock, so that
>1245 represents quater to one mid-day for eg. The second integer
>represents a time duration in a similar way, so that 345 represents
>three hours & 45 minutes. This duration is to be added to the first
>time, and the result printed out in the same notation, in this case
>1630 which is the time 3hours and 45 minutes after 1245.
>Typical output might be start time is 1415. Duration is 50 End time is
>1505.
>
>Here is my answer to the above question i would like to know if it
>is any good or crap constructive critisms accepted
>I am just a novice who plays around with c
>
>/* TIME2.C TO ADD TWO 24 HOUR INPUTS */
>#include<stdio .h>
>#define MAX 100
>#define MINS 60
>#define HOUR 24
>
>int main(void)
>{
> int time24,mins,hou r24,hourout;[/color]

A little white space between names would not hurt. In a larger
program, it would be better to put each in its own statement.
[color=blue]
> int hourtotal,hour, minstotal;
> char line[20];
> int time[7]; /* ARRAY TO HOLD DIFFERENT VALUES */
>
> /* SEPERATE HOURS FROM MINUTES */
> do{
> printf("Enter 24 hour start time: ");
> fgets(line,size of(line),stdin) ;
> sscanf(line,"%d ",&time24);
>
> mins = time24 % MAX;
> hour24 = time24 - mins;
> hourout = hour24 / MAX;[/color]

The way integer arithmetic truncates you could combine these two
statements into hourout = time24/MAX.

If time24 is 1245:
In your code, hour24 becomes 1200 and hourout is 12.
In my code, hourout becomes 12 directly.
[color=blue]
> time[0] = hourout; /* hours start */
> time[1] = mins; /* minutes start */
> }while(time[0] >= HOUR || time[1] >= MINS);[/color]

This will repeat the loop if the user enters an invalid time but it
doesn't tell him what he did wrong. For example, some people use 2400
to indicate midnight and would have no idea why you don't accept it.
[color=blue]
>
> do{
> printf("Enter duration: ");
> fgets(line,size of(line),stdin) ;
> sscanf(line,"%d ",&time24);
>
> mins = time24 % MAX;
> hour24 = time24 - mins;
> hourout = hour24 / MAX;
> time[2] = hourout; /* hours end */
> time[3] = mins; /* minutes end */
> }while(time[2] >= HOUR || time[3] >= MINS);
>
> /* ADD START AND END MINUTES */
> hourtotal = time[1] + time[3];[/color]

hourtotal is a strange name for a calculation involving minutes.
[color=blue]
> if(hourtotal >= MINS)
> {
> hour = 1;
> minstotal = hourtotal - MINS;
> }
> else
> {
> hour = 0;
> minstotal = hourtotal;
> }
>
> /* CALCULATE AND DISPLAY 24 HOUR TIME */
> time[5] = hour;
> time[6] = (time[0] + time[2] + time[5]) * MAX ;
> if(time[6] > (HOUR * MAX))[/color]

As a matter of style, indent the range of an if (which you do) but
don't indent the if itself. When you start writing larger programs,
this will make life much easier.[color=blue]
> {
> time[7] = (time[6] - (HOUR * MAX)) + minstotal;
> printf("1 Day 24 hour time output = %d\n",time[7]);
> }
> else
> {
> time[7] = time[6] + minstotal;
> printf("24 hour Time output = %d \n",time[7]);
> }
> return 0;
>}[/color]

<<Remove the del for email>>

**Darklight** · Nov 14 '05, 11:56 AM

Re: any good

Barry Schwarz wrote:>>[color=blue][color=green]
>> mins = time24 % MAX;
>> hour24 = time24 - mins;
>> hourout = hour24 / MAX;[/color]
>
> The way integer arithmetic truncates you could combine these two
> statements into hourout = time24/MAX.
>
> If time24 is 1245:
> In your code, hour24 becomes 1200 and hourout is 12.
> In my code, hourout becomes 12 directly.[/color]

Thanks for that[color=blue]
>[color=green]
>> time[0] = hourout; /* hours start */
>> time[1] = mins; /* minutes start */
>> }while(time[0] >= HOUR || time[1] >= MINS);[/color]
>
> This will repeat the loop if the user enters an invalid time but it
> doesn't tell him what he did wrong. For example, some people use 2400
> to indicate midnight and would have no idea why you don't accept it.
>[/color]
To print error message i have inserted a function here
and changed hourout to hours:

time[0] = hours; /* hours start */
time[1] = mins; /* minutes start */
ErrorPrint(hour s,mins);[color=blue][color=green]
>> }while(time[0] >= HOUR || time[1] >= MINS);[/color][/color]

The function is:
int ErrorPrint(int hour, int min)
{
if(hour >= HOUR || min >= MINS)
printf("Error incorrect value: Enter value less than 2400\n");
return(hour,min );
}

**Barry Schwarz** · Nov 14 '05, 11:57 AM

Re: any good

On Sat, 21 Aug 2004 18:48:10 +0000 (UTC), Darklight
<nglennglen@net scape.net> wrote:
[color=blue]
>Barry Schwarz wrote:>>[color=green][color=darkred]
>>> mins = time24 % MAX;
>>> hour24 = time24 - mins;
>>> hourout = hour24 / MAX;[/color]
>>
>> The way integer arithmetic truncates you could combine these two
>> statements into hourout = time24/MAX.
>>
>> If time24 is 1245:
>> In your code, hour24 becomes 1200 and hourout is 12.
>> In my code, hourout becomes 12 directly.[/color]
>
>Thanks for that[color=green]
>>[color=darkred]
>>> time[0] = hourout; /* hours start */
>>> time[1] = mins; /* minutes start */
>>> }while(time[0] >= HOUR || time[1] >= MINS);[/color]
>>
>> This will repeat the loop if the user enters an invalid time but it
>> doesn't tell him what he did wrong. For example, some people use 2400
>> to indicate midnight and would have no idea why you don't accept it.
>>[/color]
>To print error message i have inserted a function here
>and changed hourout to hours:
>
> time[0] = hours; /* hours start */
> time[1] = mins; /* minutes start */
> ErrorPrint(hour s,mins);[color=green][color=darkred]
>>> }while(time[0] >= HOUR || time[1] >= MINS);[/color][/color]
>
>The function is:
>int ErrorPrint(int hour, int min)
>{
> if(hour >= HOUR || min >= MINS)
> printf("Error incorrect value: Enter value less than 2400\n");
> return(hour,min );[/color]

What do you thing this does? Rest assured it doesn't.
[color=blue]
>}[/color]

<<Remove the del for email>>

**Darklight** · Nov 14 '05, 11:57 AM

Re: any good

Barry Schwarz wrote:>>>> }while(time[0] >= HOUR || time[1] >= MINS);[color=blue][color=green]
>>
>>The function is:
>>int ErrorPrint(int hour, int min)
>>{
>> if(hour >= HOUR || min >= MINS)
>> printf("Error incorrect value: Enter value less than 2400\n");
>> return(hour,min );[/color]
>
> What do you thing this does? Rest assured it doesn't.[/color]

It does do what it's meant to, not unless you see something i don't

**Jens.Toerring@physik.fu-berlin.de** · Nov 14 '05, 11:57 AM

Re: any good

Darklight <nglennglen@net scape.net> wrote:[color=blue]
> Barry Schwarz wrote:>>>> }while(time[0] >= HOUR || time[1] >= MINS);[color=green][color=darkred]
>>>
>>>The function is:
>>>int ErrorPrint(int hour, int min)
>>>{
>>> if(hour >= HOUR || min >= MINS)
>>> printf("Error incorrect value: Enter value less than 2400\n");
>>> return(hour,min );[/color]
>>
>> What do you thing this does? Rest assured it doesn't.[/color][/color]
[color=blue]
> It does do what it's meant to, not unless you see something i don't[/color]

You can't return two values, i.e.

return(hour,min );

will just return min, not hours. 'return' is not a function, even if
you put everything after it in (superfluous) parentheses, so the
comma here acts as the comma operator - and the comma operator makes
the program discard hour and return min.

Regards, Jens
--
\ Jens Thoms Toerring ___ Jens.Toerring@p hysik.fu-berlin.de
\______________ ____________ http://www.toerring.de

**Darklight** · Nov 14 '05, 11:57 AM

Re: any good

Jens.Toerring@p hysik.fu-berlin.de wrote:
[color=blue]
> Darklight <nglennglen@net scape.net> wrote:[color=green]
>> Barry Schwarz wrote:>>>> }while(time[0] >= HOUR || time[1] >= MINS);[color=darkred]
>>>>
>>>>The function is:
>>>>int ErrorPrint(int hour, int min)
>>>>{
>>>> if(hour >= HOUR || min >= MINS)
>>>> printf("Error incorrect value: Enter value less than 2400\n");
>>>> return(hour,min );
>>>
>>> What do you thing this does? Rest assured it doesn't.[/color][/color]
>[color=green]
>> It does do what it's meant to, not unless you see something i don't[/color]
>
> You can't return two values, i.e.
>
> return(hour,min );
>
> will just return min, not hours. 'return' is not a function, even if
> you put everything after it in (superfluous) parentheses, so the
> comma here acts as the comma operator - and the comma operator makes
> the program discard hour and return min.
>
> Regards, Jens[/color]

so would it be better to change the function to void
i just done that and it works, is that the right thing to do
thanks for your comments

**Old Wolf** · Nov 14 '05, 11:59 AM

Re: any good

Darklight <nglennglen@net scape.net> wrote:[color=blue]
>
> To print error message i have inserted a function here
> and changed hourout to hours:
>
> time[0] = hours; /* hours start */
> time[1] = mins; /* minutes start */
> ErrorPrint(hour s,mins);[color=green][color=darkred]
> >> }while(time[0] >= HOUR || time[1] >= MINS);[/color][/color][/color]
^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^ ^^^^^^^^^[color=blue]
>
> The function is:
> int ErrorPrint(int hour, int min)
> {
> if(hour >= HOUR || min >= MINS)[/color]
^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^ ^^^^^^^^^[color=blue]
> printf("Error incorrect value: Enter value less than 2400\n");
> return(hour,min );
> }[/color]

You have "code duplication" here which is generally a bad idea; you
could make the error function return a value to indicate whether there
was an error, eg:

do ... while (error_check(ho urs, mins));

int error_check(int hours, int mins)
{
if(hours < HOUR && mins < MINS)
return 0;

printf("Error incorrect value: Enter value less than 2400\n");
return 1;
}

Also, there's no point in assigning time[0] and time[1] until the
loop has actually exited (because if the loop goes around again
then their values were wasted).

any good

any good

Comment

Comment

Comment

Comment

Comment

Comment

Comment