please comment

[amanayin]

As i am not at school for c programming can you please
tell me if there is anything bad about this program

/* EX4-9.C FUNCTION TO CALL OTHER FUNCTIONS */

#include<stdio.h>

float div(float a, float b);
float multi(float a, float b);
void call(void);

float a,b,c;
char z;

int main(void)
{

printf("Enter m to multiply or d to divid:");
scanf("%c",&z);
printf("Enter two numbers: ");
scanf("%f %f", &a, &b);

call();

return 0;
}
float div(float a, float b)
{
float c;

if(b == 0)
printf("The divisor should not be a zero\n");
else
c = a / b;
return c;
/* OR return(a / b); */
}

float multi(float a, float b)
{
int c;
return (c = a * b);
/* OR return(a * b); */
}

void call(void)
{

if(z == 'm')
c = multi(a,b);
else
c = div(a,b);

if(z != 'm' && z != 'd'){
c = 0;
printf("Incorrect operator\n");
}
printf("Output %f\n",c);

}

 

[T.M. Sommers]

As i am not at school for c programming can you please
tell me if there is anything bad about this program

/* EX4-9.C FUNCTION TO CALL OTHER FUNCTIONS */

#include<stdio.h>

float div(float a, float b);

Unless you have a very good reason, such as using huge data sets,
you should use double instead of float. The few bytes you save
by using floats are not worth the trouble, and in many situations
the floats are converted to doubles anyway.

float multi(float a, float b);
void call(void);

float a,b,c;
char z;

Global variables should be avoided unless there is a very good
reason to use them. In this program, the use of globals is not
justified.

int main(void)
{

printf("Enter m to multiply or d to divid:");
scanf("%c",&z);

It is safer to use a combination of fgets and sscanf (or atof)
for interactive input. Suppose, for instance, that the user
typed in 'mm' instead of just 'm'. You should also check the
return value of scanf.

printf("Enter two numbers: ");
scanf("%f %f", &a, &b);

call();

return 0;
}
float div(float a, float b)
{
float c;

if(b == 0)

In general, one should not test floating point numbers for equality.

printf("The divisor should not be a zero\n");
else
c = a / b;
return c;

If b == 0, at this point c contains garbage.

/* OR return(a / b); */

Not if b == 0.

}

float multi(float a, float b)
{
int c;

c should be float or double.

return (c = a * b);
/* OR return(a * b); */

The alternative is preferable.

}

void call(void)
{

if(z == 'm')
c = multi(a,b);
else
c = div(a,b);

Here you call div if z is anything other than 'm'.

if(z != 'm' && z != 'd'){
c = 0;
printf("Incorrect operator\n");
}

This check should come earlier, or be made part of the if/else above.

 

[Ed Morton]

As i am not at school for c programming can you please
tell me if there is anything bad about this program

<snip>

In addition to comments you already received:

Make the body of "call()" a switch for clarity (your bug where you call
"div()" even if "c" is not 'd' wouldn't have happened if you had a
switch I bet).

Return a success/fail indication from "call()" to "main()" so it can
likewise return success/fail to the OS.

Use more meaningful names for variables (e.g. "numerator" and
"denominator" in "div()" instead of "a" and "b", "result" instead of "c"
everywhere, "operand" instead of "z" everywhere).

Delcare variables one per-line for clairty (contrast what you might
think "char *a,b;" means with what it actually means - "char *a; char b;")

No big deal, but from main() you could return EXIT_SUCCESS or
EXIT_FAILURE instead of 0 or 1 - include stdlib.h if you want to use them.

Something to consider - how will you detect it and what will you do
about it if the result of your calculation in "multi()" overflows (i.e.
is larger than the maximum value that can be held by) the type "float"?
Right now you'll just charge ahead with the calulation and return a
value that may be less than either original number.

Regards,

Ed.

 

[Sheldon Simms]

The function will return more predictable results if you
#include <math.h> and then initialize c:

float c = INFINITY;

or

float c = HUGE_VALF;

In general, one should not test floating point numbers for equality.

I think this is a poor comment. It is true "in general". But in this
code a direct comparison to zero is the correct thing to do, so why
confuse the issue? It would be better, in my opinion, to compare against
an explicitly floating point value:

if (b == 0.0)

 

[Mark McIntyre]

I think this is a poor comment. It is true "in general". But in this
code a direct comparison to zero is the correct thing to do,

Why? b is user input. Who says the io routine stores zero to perfect
precision? What if the OP thinks that comparing to zero is always ok,
and then tries to do it when b is the result of a computation?

It would be better, in my opinion, to compare against
an explicitly floating point value:

FWIW, b is a float, so 0 is promoted before the comparison, using the
usual promotion rules.

 

[Sheldon Simms]

Why? b is user input. Who says the io routine stores zero to perfect
precision?

Good question. The only applicable requirement in the standard that I
find after a quick search is:

7.20.1.3:
5 If the subject sequence has the hexadecimal form and FLT_RADIX is a
power of 2, the value resulting from the conversion is correctly rounded.

Then as recommended practice there is:

7.20.1.3:
9 If the subject sequence has the decimal form and at most DECIMAL_DIG
(defined in <float.h>) significant digits, the result should be correctly
rounded.

Nevertheless, it seems to me than an implementation that failed to
convert 0.0 precisely into a floating point value with value exactly
zero would be broken unless it is ok for the implementation to not
be able to represent 0.0 exactly at all. I don't believe that is
ok, but I haven't been able to convince myself of it 100% in the
last 5 minutes.

What if the OP thinks that comparing to zero is always ok,
and then tries to do it when b is the result of a computation?

That would be a problem, but we have no evidence of that in the
OP's post.

FWIW, b is a float, so 0 is promoted before the comparison, using the
usual promotion rules.

Obviously. I should have made it clear that I meant better style, not
"more correct".

 

[Peter Nilsson]

The function will return more predictable results if you
#include <math.h> and then initialize c:

float c = INFINITY;

 

[Sheldon Simms]

ICBW, but I don't think INFINITY was required for <math.h> in C90.

That may be true but the current standard is C99.

Personally I don't know if either of the two macros are required
in C90. I know that both are required in C99 and that HUGE_VALF

 

[Richard Bos]

Why? b is user input.

Because in context, that test is used to prevent division by zero. It
doesn't matter if you divide by user-entered-almost-zero. It is exactly
0.0 that you need to avoid dividing by, nothing else.

Richard

 

[Dan Pop]

The function will return more predictable results if you
#include <math.h> and then initialize c:

float c = INFINITY;

or

float c = HUGE_VALF;

Neither of them is part of the commonly implemented C standard.
Using them is a guaranteed recipe for portability headaches. And last
time I checked, this newsgroup was focused on portable C programming.

Dan

 

[Dan Pop]

That may be true but the current standard is C99.

Personally I don't know if either of the two macros are required
in C90. I know that both are required in C99 and that HUGE_VALF
actually appears in <math.h> on my system (with the value 'Inf').

Invoke your compiler in conforming mode and HUGE_VALF is gone from
<math.h> (or your implementation is broken).

Dan

 

[Richard Bos]

Because in context, that test is used to prevent division by zero. It
doesn't matter if you divide by user-entered-almost-zero. It is exactly
0.0 that you need to avoid dividing by, nothing else.

Especially given that floats were being used, you can overflow the
result as well as div/0, even for relatively small values of b.[/QUOTE]

Yes, but that can't be helped at all, unless you forbid all divisors
smaller than 1.0. After all, (0.9*FLT_MAX) / 0.8 overflows, as well.

Richard

 

[Mark McIntyre]

Why? b is user input.

Because in context, that test is used to prevent division by zero. It
doesn't matter if you divide by user-entered-almost-zero. It is exactly
0.0 that you need to avoid dividing by, nothing else.[/QUOTE]

Especially given that floats were being used, you can overflow the
result as well as div/0, even for relatively small values of b.

 

[Mark McIntyre]

Especially given that floats were being used, you can overflow the
result as well as div/0, even for relatively small values of b.

Yes, but that can't be helped at all, unless you forbid all divisors
smaller than 1.0. After all, (0.9*FLT_MAX) / 0.8 overflows, as well.[/QUOTE]

Agreed. However IMO the dangerous floating point comparison renders
overflows more likely - user input is likely to be a smallish number,
dividing by something almost but not entirely unlike zero would
probably still dork out.

 

[Sheldon Simms]

Invoke your compiler in conforming mode and HUGE_VALF is gone from
<math.h> (or your implementation is broken).

Nope, still works:

[sheldon@wsxyz mcc]$ cat test.c
#include <math.h>
#include <stdio.h>

int main (void)
{
printf("%g\n", HUGE_VALF);
return 0;
}
[sheldon@wsxyz mcc]$ gcc -Wall -W -O2 -std=c99 -pedantic test.c
[sheldon@wsxyz mcc]$ ./a.out
inf
[sheldon@wsxyz mcc]$

 

[av]

hi all,

T.M.Sommers mentioned "Unless you have a very good reason, such as
using huge data sets, you should use double instead of float". But
when i used double, i didnt get the result. for eg., when i multiply 2
by 3 my answer was 64.

Another doubt i have is
1. how do i find if my code is efficient and meets the performance
requirements.?
2. what is the necessity for setting value to INFINITY && HUGE_VALF ?

my code is as follows:

/* header files */
#include <stdio.h>
#include <stdlib.h>
#include <float.h>

/* function declarations */
double div(double num, double den);
double multi(double num1, double num2);

/* function main() */
int main(void)
{
// charater declaration
char choice;

// double declaration
double mulAns; // variable to store multiplication
answer
double divAns; // variable to store division answer
double num1, num2;
// function code
system("clear"); // clears the screen
printf(" Enter 'm' to Multiply or 'D' to Divide :");
scanf("%c",&choice);

if(choice=='m' || choice=='M')
{
printf("\n Enter 2 Numbers :");
scanf("%f %f",&num1,&num2);
mulAns= multi(num1,num2);
printf("\n Multiplication Answer : %f",mulAns);
printf("\n");
}
else if(choice=='d' || choice=='D')
{
printf("\n Enter Numerator && Denominator :");
scanf("%f %f",&num1, &num2);
divAns=div(num1,num2);
printf("\n Division Answer : %f",divAns);
printf("\n");
}
else if(choice !='m' || choice!='M' || choice!='d'||
choice!='D')
{
printf("\n Wrong Choice");
printf("\n");
return 1;
exit(1); // exit with failure
}

// function code ends here
return 0;
exit(0); // exit with Success
}

// function multi
double multi(double num1,double num2)
{
// double declarations
double answer;

// function code starts here
// if any 1 of the input number is zero, answer is
zero
if (num2==0.0 || num1==0.0)
{
answer=0.0;
return answer;
}
else
{
answer=num1 * num2;
return answer;
}
}

// function div
double div(double num, double den)
{
// float declarations
double answer;

// function code starts here
// if denominator is zero, the answer is zero
if(den==0.0)
{
answer=0.0;
return answer;
}
else
{
answer=num/den;
return answer;
}
}

thank you,
amsa.

 

[Mark McIntyre]

Incorrect - AFAIR an implementation is not broken because it accepts
nonstandard macros, even if in maximum conformance mode. If this were
the case, then user code would be uncompilable.

What an implementation may /not/ do in conforming mode is refuse to
accept Standard macros, or give a nonstandard meaning to them.

Nope, still works:

Probably because your implementation is the one whose writers lobbied
for HUGE_VALx to be added to the Standard

 

[Mark McIntyre]

hi all,

T.M.Sommers mentioned "Unless you have a very good reason, such as
using huge data sets, you should use double instead of float". But
when i used double, i didnt get the result. for eg., when i multiply 2
by 3 my answer was 64.

Actually, it could also have printed 0, or a zillion, or -nan or
"hello sailor".

You need to check on the correct format specifiers for printf for
printing doubles, and for scanf for reading them. Top hint: they're
NOT the same, and at least one of them is wrong in your sample code.

Also, don't use div() as a function name - its a reserved word in C as
its the name of a function in math.h

Another doubt i have is
1. how do i find if my code is efficient and meets the performance
requirements.?

Run it a million times, time it and see if its fast enough. Seriously.

2. what is the necessity for setting value to INFINITY && HUGE_VALF ?

c is a local variable, so it is not initialised to anything sensible.
If the scanf failed (eg the user typed something invalid) then c would
be left indeterminate, and your code could behave peculiarly - for
example it has a 4 in 256 chance of actually executing, even though
the user typed in "x" or "111" or just hit the enter key.

Note that you will probably never see this when debugging - most
debuggers zero all local variables, for some reason. So this will only
happen when you run it for your teacher... :-(

..

 

[Sheldon Simms]

Incorrect - AFAIR an implementation is not broken because it accepts
nonstandard macros, even if in maximum conformance mode. If this were
the case, then user code would be uncompilable.

More importantly, HUGE_VALF *is* actually a standard macro, so when
I compile with the flag "-std=c99" and it works, this is only to
be expected.

As an aside, if I compile with "-std=c89", HUGE_VALF is not recognized
as a valid macro, and if I compile with "-traditional" then the keyword
'const' isn't recognized. But since C99 is the current standard, these
bits of trivia are uninteresting, IMO.

 

[Keith Thompson]

More importantly, HUGE_VALF *is* actually a standard macro, so when
I compile with the flag "-std=c99" and it works, this is only to
be expected.

As an aside, if I compile with "-std=c89", HUGE_VALF is not recognized
as a valid macro, and if I compile with "-traditional" then the keyword
'const' isn't recognized. But since C99 is the current standard, these
bits of trivia are uninteresting, IMO.

Dan Pop's point, I think, is that the C99 standard is not yet widely
implemented, and code that depends on it is therefore non-portable and
not suitable for discussion in this newsgroup. Dan, please correct me
if I've misrepresented your position.

It's true that HUGE_VALF is required in C99; it may also be provided
(in a non-conforming mode) by some C90 implementations that have added
partial C99 support.

It's also true that a strict C90 compiler, invoked in conforming mode,
is not allowed to declare a HUGE_VALF macro in <math.h>. Otherwise
it would reject the following strictly conforming C90 program:

#include <math.h>
int main (void)
{
int HUGE_VALF;
return 0;
}