Velocity Reviews > any good

# any good

Darklight
Guest
Posts: n/a

 08-21-2004
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,hour24,hourout;
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,sizeof(line),stdin);
sscanf(line,"%d",&time24);

mins = time24 % MAX;
hour24 = time24 - mins;
hourout = hour24 / MAX;
time[0] = hourout; /* hours start */
time[1] = mins; /* minutes start */
}while(time[0] >= HOUR || time[1] >= MINS);

do{
printf("Enter duration: ");
fgets(line,sizeof(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];
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))
{
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;
}

Barry Schwarz
Guest
Posts: n/a

 08-21-2004
On Sat, 21 Aug 2004 13:06:35 +0000 (UTC), Darklight
<(E-Mail Removed)> wrote:

>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,hour24,hourout;

A little white space between names would not hurt. In a larger
program, it would be better to put each in its own statement.

> 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,sizeof(line),stdin);
> sscanf(line,"%d",&time24);
>
> mins = time24 % MAX;
> hour24 = time24 - mins;
> hourout = hour24 / MAX;

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.

> time[0] = hourout; /* hours start */
> time[1] = mins; /* minutes start */
> }while(time[0] >= HOUR || time[1] >= MINS);

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.

>
> do{
> printf("Enter duration: ");
> fgets(line,sizeof(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];

hourtotal is a strange name for a calculation involving minutes.

> 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))

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.
> {
> 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;
>}

<<Remove the del for email>>

Darklight
Guest
Posts: n/a

 08-21-2004
Barry Schwarz wrote:>>
>> mins = time24 % MAX;
>> hour24 = time24 - mins;
>> hourout = hour24 / MAX;

>
> 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.

Thanks for that
>
>> time[0] = hourout; /* hours start */
>> time[1] = mins; /* minutes start */
>> }while(time[0] >= HOUR || time[1] >= MINS);

>
> 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.
>

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(hours,mins);
>> }while(time[0] >= HOUR || time[1] >= MINS);

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
Guest
Posts: n/a

 08-22-2004
On Sat, 21 Aug 2004 18:48:10 +0000 (UTC), Darklight
<(E-Mail Removed)> wrote:

>Barry Schwarz wrote:>>
>>> mins = time24 % MAX;
>>> hour24 = time24 - mins;
>>> hourout = hour24 / MAX;

>>
>> 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.

>
>Thanks for that
>>
>>> time[0] = hourout; /* hours start */
>>> time[1] = mins; /* minutes start */
>>> }while(time[0] >= HOUR || time[1] >= MINS);

>>
>> 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.
>>

>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(hours,mins);
>>> }while(time[0] >= HOUR || time[1] >= MINS);

>
>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.

>}

<<Remove the del for email>>

Darklight
Guest
Posts: n/a

 08-22-2004
Barry Schwarz wrote:>>>> }while(time[0] >= HOUR || time[1] >= MINS);
>>
>>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.

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

Jens.Toerring@physik.fu-berlin.de
Guest
Posts: n/a

 08-22-2004
Darklight <(E-Mail Removed)> wrote:
> Barry Schwarz wrote:>>>> }while(time[0] >= HOUR || time[1] >= MINS);
>>>
>>>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.

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

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 ___ http://www.velocityreviews.com/forums/(E-Mail Removed)-berlin.de
\__________________________ http://www.toerring.de

Darklight
Guest
Posts: n/a

 08-22-2004
(E-Mail Removed)-berlin.de wrote:

> Darklight <(E-Mail Removed)> wrote:
>> Barry Schwarz wrote:>>>> }while(time[0] >= HOUR || time[1] >= MINS);
>>>>
>>>>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.

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

>
> 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

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

Old Wolf
Guest
Posts: n/a

 08-23-2004
Darklight <(E-Mail Removed)> wrote:
>
> 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(hours,mins);
> >> }while(time[0] >= HOUR || time[1] >= MINS);

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> 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);
> }

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(hours, 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).