help a beginner with a basic function that should return a char

B

bpascal123

Hi,

In the code below, I'd expect the function to return : "odd" or
"even" .

Thanks,

=o=

#include <stdio.h>

int getvalue(int i) ;

int main(void)
{
int val ;
int c ;

printf("\n\nThis prg tells if it's a odd or even number. \n\n ");

printf("Enter a value: ") ;
scanf("%d", &val) ;
c = getvalue(val) ;
printf("\n\nValeur : %d\n", c ) ;
}

int getvalue(int i)
{
char *a = "odd" ;
char *b = "even" ;

if ( i % 2 )
return a ; // is odd
else
return b ; // is even
}
 
T

Tim Harig

int main(void)
{
Given your question, I am going to ignore main().
int getvalue(int i)

Note that you are returning an int here.
{
char *a = "odd" ;
char *b = "even" ;

These are local to your function so you cannot return them safely. If you
really want to return the strings then you will need to allocate space for
a return string on the heap with malloc(). Then copy one of these values
to that string. In that case you will need to change the return type of
your function to a (char*).

Better would be to create an enum type for even or odd.

enum parity {EVEN, ODD};

You could typedef the type if you wish. Then return that value from
your function:

enum parity odd_or_even(int given) {...}
if ( i % 2 )

It is a poor practice to assume absolute values for the behaviour of
conditionals and much better to state a test explicitly:

if ((i % 2) == 1) {
return a ; // is odd
else
return b ; // is even

Here you are trying to return a (char*) to a local variable:

1. You have established that your function returns an (int) not a
(char*).

2. a and b are local so their contents may no longer exist when
this function returns.
 
B

Ben Pfaff

Tim Harig said:
These are local to your function so you cannot return them
safely.

String literals have static storage duration so this is not in
itself a problem.
 
B

bert

Hi,

In the code below, I'd expect the function to return : "odd" or
"even" .

Thanks,

=o=

#include <stdio.h>

int getvalue(int i) ;

int main(void)
{
        int val ;
        int c ;

        printf("\n\nThis prg tells if it's a odd or even number. \n\n ");

        printf("Enter a value: ") ;
        scanf("%d", &val) ;
        c = getvalue(val) ;
        printf("\n\nValeur : %d\n", c ) ;

}

int getvalue(int i)
{
        char *a = "odd" ;
        char *b = "even" ;

        if ( i % 2 )
                return a ; // is odd
        else
                return b ; // is even

}

There are two principal problems:

(1a) getvalue should return a char*, not an int.
After all, it returns either a or b, both
of which are char*, so that's what its
return type ought to be.
(1b) It follows that, in main(), c should be of
type char*, not int; and in printf(), the
format string should have %s instead of %d,
and its next argument should be *c instead of c.
(2) You declare the strings "odd" and "even"
as local to getvalue(), so in principle
they may no longer exist after the return.
In some compilers they may be okay, but
it is unsafe. They should be declared
with a large enough scope.

Experienced programmers would be more likely
to rewrite getvalue() without the strings,
and simply:
return i % 2;

The return type is now int, being 0 for even
or 1 for odd. The variable c would then be
of type int, and the output statement would
be simplified to:

printf("\n\nValeur : %s\n", c ? "odd" : "even");

Much of that is a matter of taste and style,
so don't worry if you find it cryptic at
present. But with more experience, you will
come to appreciate its simplicity.
--
 
B

Ben Pfaff

bert said:
(2) You declare the strings "odd" and "even"
as local to getvalue(), so in principle
they may no longer exist after the return.

No, you are wrong. Literal strings have static storage duration.

(Why did two people in a row get this basic fact wrong?)
 
T

Tim Harig

No, you are wrong. Literal strings have static storage duration.
(Why did two people in a row get this basic fact wrong?)

1. Most likely because it is extremely rare to return a literal from a
function.

2. Because, while I am not 100% sure of the standard, I know that I am safe
assuming all local variables go out of scope without having to
check the stardard -- even if the assumption is not 100% correct.
 
D

Default User

bert said:
(2) You declare the strings "odd" and "even"
as local to getvalue(), so in principle
they may no longer exist after the return.
In some compilers they may be okay, but
it is unsafe. They should be declared
with a large enough scope.

No. The pointer variables a and b are local, but not what they point
to. Those are string literals, which have static storage duration. It's
perfectly safe to return those addresses.

As string literals should not be modified, it would be best to have the
function declared as:

const char* getvalue(int i);


That's assuming that the OP doesn't take your other suggestion to
return actual integral values, of course.




Brian
 
B

bpascal123

        1. You have established that your function returns an (int) not a
                (char*).

MY NOT EXCUSABLE FAULT, I forgot to change int to char getvalue(int i)
since i was trying to experiment different things to see if i could
get to something (day 2 with functions)

Thanks for your interest,

Very intersting post Tim, even if i don't understand all, i will read
it over and over until it takes shape.

Thanks

Pascal
 
B

Ben Pfaff

MY NOT EXCUSABLE FAULT, I forgot to change int to char getvalue(int i)
since i was trying to experiment different things to see if i could
get to something (day 2 with functions)

A char is still not correct. The return type should be char * or
const char *.

(A char can only hold a single character. A char * can point to
the beginning of an entire C string.)

You probably don't have your compiler's warning level turned up
high enough, or it would have told you about this problem.
 
B

bpascal123

You probably don't have your compiler's warning level turned up
high enough, or it would have told you about this problem.
--

Hi Ben,

The compiler is gcc and tells me (providing getvalue is char getvalue
--- but it still seems not to be right from your reply)... but back to
my compiler, when entering gcc prgname.c -o prgname.exe I get :

=================
in function 'getvalue' :
line 24 : return makes integer from pointer without a cast
line 26 : same as line 24
=================

So I think my compiler behaves fine.

Here is the actual code :

=================

#include <stdio.h>

char getvalue(int i) ;

int main(void)
{
int val ;
int c ;

printf("\n\nThis prg tells if it's a odd or even number. \n\n ");

printf("Enter a value: ") ;
scanf("%d", &val) ;
c = getvalue(val) ;
printf("\n\nValeur : %s\n", c ) ;
}

char getvalue(int i)
{
char *a = "odd" ;
char *b = "even" ;

if ( i % 2 )
return a ; // is odd
else
return b ; // is even
}

=================

Thanks for your interest,

Pascal
 
D

Default User

char getvalue(int i)
{
char *a = "odd" ;
char *b = "even" ;

if ( i % 2 )
return a ; // is odd
else
return b ; // is even
}

Not char, char*.

You aren't really paying attention to the help people are giving you.
That tends to make people grouchy.




Brian
 
B

Barry Schwarz

There are two principal problems:

(1a) getvalue should return a char*, not an int.
After all, it returns either a or b, both
of which are char*, so that's what its
return type ought to be.
(1b) It follows that, in main(), c should be of
type char*, not int; and in printf(), the
format string should have %s instead of %d,
and its next argument should be *c instead of c.

No, if you change the format to %s then the new c (a char*) is
correct.
(2) You declare the strings "odd" and "even"
as local to getvalue(), so in principle
they may no longer exist after the return.
In some compilers they may be okay, but
it is unsafe. They should be declared
with a large enough scope.

Actually they only need to be declared with sufficient storage
duration. And they are.
 
B

bpascal123

You aren't really paying attention to the help people are giving you.
That tends to make people grouchy.

Brian

Hi Brian,

I was focysing on the compiler behavior from a previous post. However,
even with * it still wouldn't work.

Pascal
 
B

Beej Jorgensen

I was focysing on the compiler behavior from a previous post. However,
even with * it still wouldn't work.

You'll have to be more specific. I took your code and put char * in all
the right places and it worked for me.

Are you still getting warnings about converting pointers to integers?

-Beej
 
B

Beej Jorgensen

Ben Pfaff said:
(Why did two people in a row get this basic fact wrong?)

Probably part of it has to do with:

char a[] = "foobar";
char *b = "foobar";

-Beej
 
B

Ben Pfaff

Beej Jorgensen said:
Ben Pfaff said:
(Why did two people in a row get this basic fact wrong?)

Probably part of it has to do with:

char a[] = "foobar";
char *b = "foobar";

For what it's worth, it's important to be familiar with the C FAQ
if you're posting here. This issue is discussed in the very
first section:

1.32: What is the difference between these initializations?

char a[] = "string literal";
char *p = "string literal";

My program crashes if I try to assign a new value to p.

A: A string literal can be used in two slightly different ways. As
an array initializer (as in the declaration of char a[]), it
specifies the initial values of the characters in that array.
Anywhere else, it turns into an unnamed, static array of
characters, which may be stored in read-only memory, which is
why you can't safely modify it. In an expression context, the
array is converted at once to a pointer, as usual (see section
6), so the second declaration initializes p to point to the
unnamed array's first element.

(For compiling old code, some compilers have a switch
controlling whether strings are writable or not.)

See also questions 1.31, 6.1, 6.2, and 6.8.

References: K&R2 Sec. 5.5 p. 104; ISO Sec. 6.1.4, Sec. 6.5.7;
Rationale Sec. 3.1.4; H&S Sec. 2.7.4 pp. 31-2.
 
P

praveen21

In the code below, I'd expect the function to return : "odd" or
"even" .


#include <stdio.h>
int getvalue(int i) ;
int main(void)
{
   int val ;
   int c ;
   printf("\n\nThis prg tells if it's a odd or even number. \n\n ");
   printf("Enter a value: ") ;
   scanf("%d", &val) ;
   c = getvalue(val) ;
   printf("\n\nValeur : %d\n", c ) ;
}
int getvalue(int i)
{
   char *a = "odd" ;
   char *b = "even" ;
   if ( i % 2 )
           return a ; // is odd
   else
           return b ; // is even
}

#include <stdio.h>

char *getvalue(unsigned u);

int main(void)
{
     unsigned val;

     printf("\n\nThis prg tells if it's a odd or even number. \n\n ");
     printf("Enter a value: ");
     if (scanf("%u", &val) == 1) {
         printf("\n\nValeur : %s\n", getvalue(val));
     }
     return 0;

}

char * getvalue(unsigned u)
{
     char *a[] ={"even","odd"};

     return a[u % 2];

}

perfect code
 
B

Ben Bacarisse

praveen21 said:
On Jul 11, 7:40 am, pete <[email protected]> wrote:
char * getvalue(unsigned u)
{
     char *a[] ={"even","odd"};

     return a[u % 2];

}

perfect code

I don't want to sound churlish (and, unprompted, I would not have said
a word about this fine posting) but "perfect" is a strong word to use.

Did you notice that pete (who, quite reasonably, wants to write
portable C90 code) has had to chance the specification of the function
so that now it takes an unsigned int? Do you know why? I am pretty
sure it is simply so that indexing the array with u % 2 is always 0 or
1. If u were a signed int, the result on some systems can be -1. Of
course he could have written:

char *a[] = {"even", "odd"};
return a[abs(i % 2)]; /* i now signed int again */

or used a three-element array indexed with a % 2 + 1, but the original
is neater. This kind of subtlety -- the need to use unsigned types --
is the kind of detail that merits a comment.

On a related issue, if a function had to return one of two numbers
based some binary condition, would you put them into an array and then
index the array:

double either[] = {2.71828, 3.14159};
return either[i % 2];

? Personally, I wouldn't. I'd use a conditional expression, and I'd
do the same in the original case.

Finally, I'd prefer that the return type of the function be 'const
char *' and that it have a better name, although neither may be
permitted if the specification is fixed.

const char *parity(int i)
{
return i % 2 ? "odd" : "even";
}

Of course, pete never claimed (and I am certain he never would claim)
that his code was perfect; and I certainly don't make that claim for
my silly one liner, but it seems worth posting if only to show that
there is more to learn by having a debate about code than by labelling
it.
 
B

bpascal123

Mouse shut for those latest post !

It either totally matches what i was looking for in my learning
process and goes above with additional assertions that will be very
useful to take in consideration later on.

Thx
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,756
Messages
2,569,534
Members
45,007
Latest member
OrderFitnessKetoCapsules

Latest Threads

Top