«

Question the program below is taken from a book which i have been
told is bad, but it gives me the basics which is what i need, and
yes i know the gets function should not be used, but i would like
to see how other people would get the program below to work.

I have already got it to work by changing some thing in the
typedef struct, but i would like to see how the more experienced
programmer would get it to work

The program is used to demonstrate "unions in structures"
the book it's taken from is "c programming in easy steps"

#include<stdio.h>

typedef struct
{
union{ int num; char letter; };
char *name;
} info;

int main(void)
{
info stored; /* create a struct of the info type */
printf("Please enter your first name: ");
gets(stored.name);
printf("Enter a number to convert to hex? [Y or N]: ");
scanf("%c",&stored.letter);
if(stored.letter == 'y' || stored.letter == 'Y')
{
printf("OK, enter the number to be converted; ");
scanf("%d",&stored.num);
printf("Thanks %s, ",stored.name);
printf("%d in hex is 0x%X\n", stored.num, stored.num);
}
return 0;
}

Darklight said:

> Question the program below is taken from a book which i have been
> told is bad, but it gives me the basics which is what i need, and
> yes i know the gets function should not be used, but i would like
> to see how other people would get the program below to work.

I would start by removing the call to gets().

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)

Richard Heathfield <> wrote:

> Darklight said:
>
> > Question the program below is taken from a book which i have been
> > told is bad, but it gives me the basics which is what i need, and
> > yes i know the gets function should not be used, but i would like
> > to see how other people would get the program below to work.
>
> I would start by removing the call to gets().

And then rip the union out. No need for it.

Richard

Darklight wrote:
> Question the program below is taken from a book which i have been
> told is bad, but it gives me the basics which is what i need, and

If it is bad it probably gives you incorrect basics and leads you in to
doing things wrong.

> yes i know the gets function should not be used, but i would like
> to see how other people would get the program below to work.
>
> I have already got it to work by changing some thing in the
> typedef struct, but i would like to see how the more experienced
> programmer would get it to work
>
> The program is used to demonstrate "unions in structures"
> the book it's taken from is "c programming in easy steps"

I would say it is doing a bad job of that.

> #include<stdio.h>
>
> typedef struct
> {
> union{ int num; char letter; };

I would not bother with this union. For this program a local variable
for the response would be better. If you were using the union for
something sensible, then you would also want to store some indication of
what type of data you had put in it, otherwise you won't know how to get
it out correctly.

> char *name;

This is a pointer, unless you make this point somewhere useful before
you use it anything could happen.

> } info;
>
> int main(void)
> {
> info stored; /* create a struct of the info type */
> printf("Please enter your first name: ");

You need to flush stdout to stand a reasonable chance of the prompt
being displayed. Either that or terminate it with a new line. The reason
being that terminal IO is generally line buffered (as allowed by the
standard) and so the output does not occur until a new line (or it gets
flushed).

> gets(stored.name);

Apart from gets which you know is wrong (use fgets) you have just told
gets to put the data in some random location (it's actually worse than
that). You need to make stored.name point somewhere useful first where
there is enough space to store a name. Either that or change the type of
stored.name in to an array (which is probably what I would do).

Also, check the return value of any input function you use. The user
could have generated an "end-of-file" (yes, you can do that from the
keyboard on many systems) in which case you won't have read a name and
subsequent reads will not work.

> printf("Enter a number to convert to hex? [Y or N]: ");

See comment above about last prompt.

> scanf("%c",&stored.letter);

scanf is not an easy function to use correctly, especially when it comes
to validating user input. I would use fgets to read a line then process it.

> if(stored.letter == 'y' || stored.letter == 'Y')

You might want to look up toupper and tolower, then can simplify things.

> {
> printf("OK, enter the number to be converted; ");
> scanf("%d",&stored.num);

what if the user enters "fred"? Again, using fgets to read the line then
processing it after would be better.

> printf("Thanks %s, ",stored.name);
> printf("%d in hex is 0x%X\n", stored.num, stored.num);
> }

By this point you don't know whether stored.letter or stored.num was the
last thing to be written, so the data is effectively useless.

> return 0;
> }

I don't think I would write a program anything like that, and certainly
not for demonstrating anything to do with unions.
--
Flash Gordon
Living in interesting times.
Although my email address says spam, it is real and I read it.

Darklight!

> Question the program below is taken from a book which i have been
> told is bad, but it gives me the basics which is what i need

It doesn't. It is a particularily bad example of a union within a struct,
the char *name is never intialized to point to something meaningful which
is a major bug, it uses gets which is very basic but also completely and
utterly useless due to its limitations, it uses scanf in a pretty stupid
way and it uses scanf again it a fairly stupid way that might not work at
all (depending on circumstances).

> I have already got it to work by changing some thing in the
> typedef struct, but i would like to see how the more experienced
> programmer would get it to work

That would presumably be changing char *name to char name[...] or
something of the sort.

I'd use fgets instead of gets and specify the appropriate buffer size.

I'd not use scanf to read a single character. Actually, I wouldn't use
scanf at all; I'd prefer fgets and then some means of parsing (including,
possibly, sscanf with two s).

Oh yeah, and I'd kick the union out: It is a perfect example of when you
should NOT use a union. You don't use unions to save storage space.


Daniel

Daniel Fischer wrote:

> You don't use unions to save storage space.

That's the way they are used
when the topic of unions is introduced in K&R.

"This is the purpose of a union
- single variable that can legitimately hold one of several types."

--
pete

Daniel Fischer wrote:

> Oh yeah, and I'd kick the union out: It is a perfect example of when you
> should NOT use a union. You don't use unions to save storage space.

Well, I can see two uses for unions: type punning (which is not portable
except in the cases where you can do it with judicious pointer casting,
as far as I'm aware) and saving space. The latter seems more frequent
than the former.

Or am I missing something?

--
Chris "straight through Friar Tuck's hat!" Dollin
"Certainly the absence of a smiley is not a syntax error or a constraint
violation, and therefore doesn't require a diagnostic." (Richard
Heathfield)

Chris Dollin wrote:
> Daniel Fischer wrote:
>
>
>>Oh yeah, and I'd kick the union out: It is a perfect example of when you
>>should NOT use a union. You don't use unions to save storage space.
>
>
> Well, I can see two uses for unions: type punning (which is not portable
> except in the cases where you can do it with judicious pointer casting,
> as far as I'm aware) and saving space. The latter seems more frequent
> than the former.
>
> Or am I missing something?
>

I suppose you could do function overloading without resorting
to void *. I don't know how worthwhile it would be.

--
imalone

Chris Dollin <> wrote:

> Daniel Fischer wrote:
>
> > Oh yeah, and I'd kick the union out: It is a perfect example of when you
> > should NOT use a union. You don't use unions to save storage space.
>
> Well, I can see two uses for unions: type punning (which is not portable
> except in the cases where you can do it with judicious pointer casting,
> as far as I'm aware) and saving space. The latter seems more frequent
> than the former.
>
> Or am I missing something?

Abstract data types with variant records.

Richard

Darklight <> writes:
> Question the program below is taken from a book which i have been
> told is bad, but it gives me the basics which is what i need, and
> yes i know the gets function should not be used, but i would like
> to see how other people would get the program below to work.
>
> I have already got it to work by changing some thing in the
> typedef struct, but i would like to see how the more experienced
> programmer would get it to work
>
> The program is used to demonstrate "unions in structures"
> the book it's taken from is "c programming in easy steps"

Amazon shows a book by that title written by Mike McGrath. (I was
expecting it be by Schildt, but even Schildt's code isn't usually
*quite* this bad.)

Amaxon has one review of the book, giving it 5 stars and describing it
as "a quick and easy to understand book for learning C++". I wouldn't
trust a reviewer who doesn't even notice what language a book covers.

accu.org has two reviews of the book, one by Pete Goodliffe at
<http://www.accu.org/cgi-bin/accu/rvout.cgi?from=0au_m&file=c003350a>
and one by Francis Glassborow at
<http://www.accu.org/bookreviews/public/reviews/c/c003566.htm>.
Though I haven't seen the book, I strongly suspect the latter review
is more accurate.

> #include<stdio.h>
>
> typedef struct
> {
> union{ int num; char letter; };
> char *name;
> } info;
>
> int main(void)
> {
> info stored; /* create a struct of the info type */
> printf("Please enter your first name: ");
> gets(stored.name);
> printf("Enter a number to convert to hex? [Y or N]: ");
> scanf("%c",&stored.letter);
> if(stored.letter == 'y' || stored.letter == 'Y')
> {
> printf("OK, enter the number to be converted; ");
> scanf("%d",&stored.num);
> printf("Thanks %s, ",stored.name);
> printf("%d in hex is 0x%X\n", stored.num, stored.num);
> }
> return 0;
> }

Is this *really* what's printed in the book? That's just horrible.
It might be a good exercise for a more advanced book, but only if the
point is to find all the errors.

This book is teaching you bad habits that could haunt you for years,
until you unlearn them. Get rid of the book. Don't give it to
someone else who might actually use it. If you give it away, tape a
copy of Francis Glassborow's review inside the front cover.

K&R2 (_The C Programming Language_, 2nd Edition, by Kernighan &
Ritchie) is generally considered the very best C tutorial, but it's
aimed at readers who already have some programming experience. If you
need something more elementary, I'm sure someone here can recommend
something good.

--
Keith Thompson (The_Other_Keith) kst- <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.