Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > please comment

Reply
Thread Tools

please comment

 
 
amanayin
Guest
Posts: n/a
 
      11-09-2003
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);

}


 
Reply With Quote
 
 
 
 
T.M. Sommers
Guest
Posts: n/a
 
      11-09-2003
amanayin wrote:
> 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.

> printf("Output %f\n",c);
>
> }
>
>


 
Reply With Quote
 
 
 
 
Ed Morton
Guest
Posts: n/a
 
      11-09-2003


amanayin wrote:
> 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.




 
Reply With Quote
 
Sheldon Simms
Guest
Posts: n/a
 
      11-09-2003
On Sun, 09 Nov 2003 12:19:37 -0500, T.M. Sommers wrote:

> amanayin wrote:
>>
>> float div(float a, float b)
>> {
>> float c;


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

float c = INFINITY;

or

float c = HUGE_VALF;

>>
>> if(b == 0)

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

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


 
Reply With Quote
 
Mark McIntyre
Guest
Posts: n/a
 
      11-09-2003
On Sun, 09 Nov 2003 13:52:12 -0500, in comp.lang.c , Sheldon Simms
<(E-Mail Removed)> wrote:

>On Sun, 09 Nov 2003 12:19:37 -0500, T.M. Sommers wrote:
>
>>> if(b == 0)

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


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.

--
Mark McIntyre
CLC FAQ <http://www.eskimo.com/~scs/C-faq/top.html>
CLC readme: <http://www.angelfire.com/ms3/bchambless0/welcome_to_clc.html>


----== Posted via Newsfeed.Com - Unlimited-Uncensored-Secure Usenet News==----
http://www.newsfeed.com The #1 Newsgroup Service in the World! >100,000 Newsgroups
---= 19 East/West-Coast Specialized Servers - Total Privacy via Encryption =---
 
Reply With Quote
 
Sheldon Simms
Guest
Posts: n/a
 
      11-10-2003
On Sun, 09 Nov 2003 21:13:23 +0000, Mark McIntyre wrote:

> On Sun, 09 Nov 2003 13:52:12 -0500, in comp.lang.c , Sheldon Simms
> <(E-Mail Removed)> wrote:
>
>>On Sun, 09 Nov 2003 12:19:37 -0500, T.M. Sommers wrote:
>>
>>>> if(b == 0)
>>>
>>> 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,

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

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


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


 
Reply With Quote
 
Peter Nilsson
Guest
Posts: n/a
 
      11-10-2003
Sheldon Simms <(E-Mail Removed)> wrote in message news:<(E-Mail Removed)>...
....
> > amanayin wrote:
> >>
> >> float div(float a, float b)
> >> {
> >> float c;

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


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

>
> or
>
> float c = HUGE_VALF;


--
Peter
 
Reply With Quote
 
Sheldon Simms
Guest
Posts: n/a
 
      11-10-2003
On Sun, 09 Nov 2003 17:34:44 -0800, Peter Nilsson wrote:

> Sheldon Simms <(E-Mail Removed)> wrote in message news:<(E-Mail Removed)>...
> ...
>> > amanayin wrote:
>> >>
>> >> float div(float a, float b)
>> >> {
>> >> float c;

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

>
> 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
actually appears in <math.h> on my system (with the value 'Inf').

>>
>> or
>>
>> float c = HUGE_VALF;



 
Reply With Quote
 
Richard Bos
Guest
Posts: n/a
 
      11-10-2003
Mark McIntyre <(E-Mail Removed)> wrote:

> On Sun, 09 Nov 2003 13:52:12 -0500, in comp.lang.c , Sheldon Simms
> <(E-Mail Removed)> wrote:
>
> >On Sun, 09 Nov 2003 12:19:37 -0500, T.M. Sommers wrote:
> >
> >>> if(b == 0)
> >>
> >> 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,

>
> 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
 
Reply With Quote
 
Dan Pop
Guest
Posts: n/a
 
      11-10-2003
In <(E-Mail Removed)> Sheldon Simms <(E-Mail Removed)> writes:

>On Sun, 09 Nov 2003 12:19:37 -0500, T.M. Sommers wrote:
>
>> amanayin wrote:
>>>
>>> float div(float a, float b)
>>> {
>>> float c;

>
>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
DESY Zeuthen, RZ group
Email: http://www.velocityreviews.com/forums/(E-Mail Removed)
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
HOLD warning? Please comment on my code! Analog_Guy VHDL 6 06-01-2005 05:16 PM
? ELSE Conditional Comment / Using Conditional Comments Inside Other Tags To Comment Out Attributes Alec S. HTML 10 04-16-2005 02:21 AM
Re: Please comment this network<<--Tried 3 times, Post isnt showing up. firemarsh Cisco 0 01-21-2004 06:05 AM
New Module: DBIx::Config - please comment Al Tobey Perl 0 10-24-2003 09:59 PM
please help... ...me learn C++ please please please :) KK C++ 2 10-14-2003 02:08 PM



Advertisments