2D array of structures

[Keith Thompson]

Keith Thompson said: [...]

There's also no need to use different identifiers for the struct tag
and the typedef name:

typedef struct Point {
int x;
int y;
} Point;

It's true that there's no C reason to use different identifiers, but I do so
anyway because it helps Microsoft's "Intellisense" to work out what you
mean when you hit the button that says "take me to your definition". Not
that I use Microsoft C terribly often - but when I do, I usually end up
thanking myself for using a unique tag name.

[...]

Alas, it's sometimes necessary to obfuscate your code to cater to
inferior tools.

 

[Richard Heathfield]

Keith Thompson said:

Alas, it's sometimes necessary to obfuscate your code to cater to
inferior tools.

I am not convinced that a mere trailing underscore on a tag name constitutes
obfuscation.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.

 

[svata]

Hello,

I meant that I admire people who knows more than I do But, I try to
code first, and if I'm stuck I ask for help. I don't expect someone to
do coding for me.
But anyway, thanks for your patience.

svata

 

[svata]

I'm sorry for that mess.

(svata's quoting was misleading anyway, since he half-top-posted.)

So, I have to ask. I did my best, my code is almost finished, but I'm
looking for someone who can do code review and comment it. Via email
preferably.
There are some bugs, which I can't get rid of. I use both Borland C
compiler and gcc.
On windows it runs with some drawbacks, on linux it crashes due to bad
use of realloc().

So, is anyone ready for code review please?

I'd hesitate to describe myself so, since there's so much C I don't
know (most of the C99 stuff, for example!) and I've used it in so
few environments. I know /some/ stuff about C. I hope it's useful
stuff.

Is your singing so bad?

But not when I'm singing.

--

svata

 

[Richard Heathfield]

svata said:

I did my best, my code is almost finished, but I'm
looking for someone who can do code review and comment it.

Try posting it, then.

Via email preferably.

Good luck with that, but in general you'll find that the email responders
are those who fear their answers will not survive in the glare of public
scrutiny.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.

 

[Chris Dollin]

Chris Dollin said:


Think "relative". Compared to most OPs around here, you're a towering
genius!

You've met me. You know "towering" isn't in my job description.

 

[svata]

Richard Heathfield wrote:

The only problem is that the code is supposed as a school task. I do
not fear to reveal my code at all. But would like to avoid my lecturer
to find my code before I submit it.

 

[Jens Thoms Toerring]

Please don't top-post. I have rearranged your post to have what
you were replying to to come first:

The only problem is that the code is supposed as a school task. I do
not fear to reveal my code at all. But would like to avoid my lecturer
to find my code before I submit it.

First question: why can't you ask you lecturer? Isn't (s)he supposed
to teach you? Or are you supposed to have understood already every-
thing and this is a test if you did?

Next question: are you not allowed to ask others for help? If you're
allowed what's the problem with posting your code here? If you're not
allowed why should we help you cheating? But if you have some other
reason for not posting your code try to write a similar program and,
if it exhibits the same bugs, post that here and apply what you learned
from the replies to the program you have to hand in. As a bonus you may
even figure out yourself what the error is when you try to write a
similar program...
Regards, Jens

 

[CBFalconer]

Richard Heathfield wrote:

The only problem is that the code is supposed as a school task. I
do not fear to reveal my code at all. But would like to avoid my
lecturer to find my code before I submit it.

You have been asked multiple times to stop top-posting, yet you
insist on so doing. I conclude you really do not care whether or
not anyone reads your posts. I shall not be doing so.

 

[svata]

Please don't top-post. I have rearranged your post to have what
you were replying to to come first:

Ok, I remember it from now on

First question: why can't you ask you lecturer? Isn't (s)he supposed
to teach you? Or are you supposed to have understood already every-
thing and this is a test if you did?

I can, but I'm somehow supposed do my own research.

Next question: are you not allowed to ask others for help? If you're
allowed what's the problem with posting your code here? If you're not
allowed why should we help you cheating? But if you have some other
reason for not posting your code try to write a similar program and,
if it exhibits the same bugs, post that here and apply what you learned
from the replies to the program you have to hand in. As a bonus you may
even figure out yourself what the error is when you try to write a
similar program...

I don't ask anyone her to help me to cheat. I can ask for help another
people, of course.
I rather wonder if someone can comment it to help me to learn good
programming habits.

So my code follows:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define FIRST 7
#define MAX 10

// First of variables have to be declared
char answer;
char item_name[MAX];
char *p_item_name;
char string;
int i;
int j;
int str_len;
float amount;
int NEW_SIZE;


typedef struct {
// to store strig entered by user
char *name;
// to be able determine whether it is predefined menu or not, used
when receipt is printed out
int menu;
// to be able determine to which menu group
int group;
// to store price entered by user
float price;
// important as we need to see which index is already used
int used;
} SHOPPING;

SHOPPING *p_shopping;

// Declaring of menu items, necessary for later reuse in array
char *p_menu_items[] = {"Bread", "Butter", "Confectionary", "Fruit",
"Meat", "Milk", "Vegetables", "Other"};

// Declaring choices menu, not being used in any other array
char *p_menu_choices[] = {"Sub-total", "Total", "Quit"};


int main() {

int store_data(int); // declaring we want to use function
// now, allocating memory for first 8 arrays, menu is going to be
stored there
p_shopping = malloc(FIRST * sizeof *p_shopping);
if ( p_shopping == NULL ) {
printf("Failed to allocate memory, exiting...");
return 1;
}

while (1) { // infinitive loop
// This cannot be part of a menu array, to be printed once only!!
printf("\nEnter your choice: \n");
// Putting menu together by looping through 2 arrays
for ( i = 0; i < (FIRST + 1); i++ ) {
// it's time to allocate memory on the fly, as we have
different string size
p_shopping.name = malloc((strlen(p_menu_items) + 1) *
sizeof(char));
if ( p_shopping.name == NULL ) {
printf("Failed to allocate memory, exiting...");
return 1;
}
//copying content of menu items to shopping structure for later use
strcpy(p_shopping.name, p_menu_items)
// and set int used to 1, to determine which indexes are used
p_shopping.used = 1;
printf("\t\t\t %d. %s\n", (i + 1), p_shopping.name );
// as this is predefined menu, we have to state it here, for future
use
p_shopping.menu = 1;
// set int group to its value according group of items, for future
use
p_shopping.group = i;
}
// second array for menu
for ( i = 0; i < 3; i++ ) {
// Neccessary to align, because there are numbers greater than 9
if ( i < 1 ) {
printf("\t\t\t %d. %s\n", ( i + 9 ), p_menu_choices);
}
else {
printf("\t\t\t%d. %s\n", ( i + 9 ), p_menu_choices);
}
}
// This cannot be a part of menu array, to be printed once only!!
printf("\t\t\t=====> ");
scanf("%d", &choice);
getchar();


switch(choice) {
// Optional exit, user has to confirm it...
case 0 :
while (1) {
printf("Do you really want to exit? [y/n]: ");
getchar();
scanf("%c", &answer);
getchar();

switch (answer) {
case 'y':
printf("Ok, as you wish, quiting...\n");
return 0;

case 'n':
printf("Ok, returning back...\n");
// had to use go to, otherwise it is almost impossible to escape
infinite loop
goto exit;

default:
printf("Please answer 'y' or 'n'!\n");
break;
}
}

case 1 :
store_data(0);
break;

case 2 :
store_data(1);
break;

case 3 :
store_data(2);
break;

case 4 :
store_data(3);
break;

case 5 :
store_data(4);
break;

case 6 :
store_data(5);
break;

case 7 :
store_data(6);
break;

case 8 :
store_data(7);
break;

case 9 :
store_data(-1);
break;

case 10 :
store_data(-1);
printf("Enter amount tendered: ");
scanf("%f", &amount);
// to test whether amount tendered is greater than total sum
if ( amount < sub_sum ) {
printf("Tendered amount is lower than total sum!!!");
// if so, exit here and user has to enter new choice
break;
}
else {
printf("Your change is %4.2f\n\n", (amount - sub_sum));
}

case 11 :
// user wants to leave
printf("Thank you for shopping\n");
return 1;

default :
// user entered invalid choice
printf("Invalid choice entered, quiting....\n");
return 0;
}
// here points goto from switch 0
exit:
// we do nothing here, just leaving inner loop
;
}
}

int store_data(int group) {
int choice;
float sum;
float sub_sum;
static int SIZE = 0; // need to use persistent variable, which counts
invocation of function
if ( group == -1 ) {
// this is really important, otherwise it screws up the result
sub_sum = 0;
// now, receipt is expected to be printed
printf("#########################\n#\t YOUR
RECEIPT\t#\n#########################\n\n");
// as we have 8 main menu items, we want to print them, one by one
for ( j = 0; j < (FIRST + 1); j++ ) { // can use FIRST as we know
that first 8 items are menus...
printf("%s\n-------------------------------\n", p_shopping[j].name);
for ( i = 0; i < (NEW_SIZE + 1); i++ ) { // and now print relevant
submenu
if ( p_shopping.group == j && p_shopping.menu != 1 ) {
sub_sum += p_shopping.price;
printf("%s\t\t\t%4.2f\n", p_shopping.name,
p_shopping.price);
}
}
// submenu separator
printf("\n");
}
printf("------------------------------\n");
printf("Total\t\t\t%4.2f\n\n", sub_sum);
return 0;
}
else {
SIZE ++; // we want keep trace how many times is this part invoked
NEW_SIZE = SIZE + FIRST;
for ( i = 0; i < (NEW_SIZE); i++ ) {
printf("%d\t%d.%d %s\n", i, p_shopping.used, p_shopping.group,
p_shopping.name );
}
printf("Enter description (max. size %d): ", sizeof(item_name));
if (fgets(item_name, sizeof(item_name), stdin) != NULL){
if (( p_item_name = strchr(item_name, '\n')) != NULL )
*p_item_name = '\0';
}
p_shopping[NEW_SIZE].used = 0; // must be set to 0 as it can have
random value
printf("Price: ");
scanf("%f", &sum);
// it's time to resize array now, as we have data we need
p_shopping = realloc(p_shopping, (sizeof(*p_shopping) * (NEW_SIZE +
1)));
if ( p_shopping == NULL ) {
printf("Failed to allocate memory, exiting...");
return 1;
}
else {
for ( i = 0; i < (NEW_SIZE + 1); i++ ) {
if ( i == (NEW_SIZE)) {
p_shopping.name = malloc((strlen(item_name) + 1) *
sizeof(char));
strcpy(p_shopping.name, item_name);
p_shopping.used = 1;
p_shopping.price = sum;
p_shopping.group = group;
}
//printf("%d\t%d.%d %s\n", i, p_shopping.used,
p_shopping.group, p_shopping.name );
}
}
return 0;
}
}

This example has cca 250 lines, I hope it is appropriate to post here.
There is a bug in use of realloc() and so far I was unable to figure it
out. I did a lot of "debuging" by using many printf() commands at
different stages.
So please feel free to comment coding style, semantic etc. I know, this
is not the best solution, but this my attempt to find it. I'm supposed
to learn by doing.

Svata

 

[Chris Dollin]

svata wrote:

// First of variables have to be declared
char answer;
char item_name[MAX];
char *p_item_name;
char string;
int i;
int j;
int str_len;
float amount;
int NEW_SIZE;

That's rather a lot of globals for a problem this size.
Most of those look like candidates for locals. If they're
going to be globals, they should have better names.

int main() {

int store_data(int); // declaring we want to use function

Usually put /outside/ functions, not inside.

For heaven's sake, /split this code up/. Functions are /useful/ and
one of C's few structuring tools.

case 1 :
store_data(0);
break;

case 2 :
store_data(1);
break;

case 3 :
store_data(2);
break;

case 4 :
store_data(3);
break;

case 5 :
store_data(4);
break;

case 6 :
store_data(5);
break;

case 7 :
store_data(6);
break;

case 8 :
store_data(7);
break;

Duplication like this should make your blood run cold and your
hair stand on end, as if you were standing next to a Van der
Graff generator and the arcing were imminent. At least you
can write:

case 1: case 2: ... case 8: store_data( choice - 1 ); break;

(Layout to taste, I just wanted one line for the post.)

 

[Default User]

svata wrote:

I rather wonder if someone can comment it to help me to learn good
programming habits.

So my code follows:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define FIRST 7
#define MAX 10

// First of variables have to be declared

Right away, this raft of global variables raises many flags. There are
occasions when globals are the right solution, but for the most part
it's a sign of a lazy or inexperienced programmer.

char answer;
char item_name[MAX];
char *p_item_name;
char string;

"string" is declared as a single character. The name doesn't fit the
type, and is probably an error. I imagine you wanted a character buffer.

int i;
int j;
int str_len;
float amount;
int NEW_SIZE;

Try to reserve all caps for macros.

typedef struct {
// to store strig entered by user

// comments are a feature of c99, although often an available
extension. We try to avoid them here due to line-wrapping.

char *name;
// to be able determine whether it is predefined menu or not, used
when receipt is printed out
int menu;
// to be able determine to which menu group
int group;
// to store price entered by user
float price;
// important as we need to see which index is already used
int used;
} SHOPPING;

Again, don't use all-caps.

SHOPPING *p_shopping;

// Declaring of menu items, necessary for later reuse in array
char *p_menu_items[] = {"Bread", "Butter", "Confectionary", "Fruit",
"Meat", "Milk", "Vegetables", "Other"};

This should be:

const char *p[] = . . .

String literals are not modifiable.

// Declaring choices menu, not being used in any other array
char *p_menu_choices[] = {"Sub-total", "Total", "Quit"};

Same here.

int main() {

int store_data(int); // declaring we want to use function
// now, allocating memory for first 8 arrays, menu is going to be
stored there
p_shopping = malloc(FIRST * sizeof *p_shopping);

FIRST is 7, yet you say you are allocating eight. Which is it? Also,
why dynamic allocation

if ( p_shopping == NULL ) {
printf("Failed to allocate memory, exiting...");
return 1;

Use the standard return values: 0, EXIT_SUCCESS, or EXIT_FAILURE.

}

while (1) { // infinitive loop
// This cannot be part of a menu array, to be printed once only!!
printf("\nEnter your choice: \n");

You prompt for a choice, but I don't see any input statement.

// Putting menu together by looping through 2 arrays
for ( i = 0; i < (FIRST + 1); i++ ) {
// it's time to allocate memory on the fly, as we have
different string size
p_shopping.name = malloc((strlen(p_menu_items) + 1) *
sizeof(char));

p_shopping has only 7 structs in the array, with valid indexes of 0-6.
You are attempting to use up to index 7. Boom (if you're lucky).



I'm going to stop here. You have too many problems. You tried to write
the whole program at once, and that seldom works. Even experienced
programmers don't do it that way.

Start over. Start small. Build up the pieces. Get your menu selection
working first. Use functions. At the early stages, those will be stubs,
then get their code expanded.





Brian

 

[Richard Heathfield]

svata said:

I can, but I'm somehow supposed do my own research.

Consulting experts doesn't count as research?

I don't ask anyone her to help me to cheat.

None of the experts here will help you to cheat, but we will help you to
learn, if you're willing for that to happen.

I can ask for help another people, of course.

That's up to you, of course.

I rather wonder if someone can comment it to help me to learn good
programming habits.
Sure.

So my code follows:

Fabulous.

Okay, here's the result of my first compilation:

foo.c:59: unterminated character constant
foo.c:198: warning: string constant runs past end of line
foo.c:235: unterminated character constant
make: *** [foo.o] Error 1

So let's take a look at those. Here's the first:

// it's time to allocate memory on the fly, as we have
different string size

This is the preprocessor complaining about the single, unmatched apostrophe.
Yes, it's in a comment, but it's a new-style C99 comment which not all
implementations understand (and in fact implementations are obliged to
diagnose it as a syntax error unless, of course, they are C99 compilers,
which are rather rare).

As you can see from the quote, the comment also got line-wrapped by Usenet,
making it spill into a second line, which can only cause trouble when the
preprocessor finally hands over to the compiler.

My fix for this was to remove all comment lines completely.

Here are line 198 and 199:

printf("#########################\n#\t YOUR
RECEIPT\t#\n#########################\n\n");

My fix for this is as follows:

printf("#########################\n#\t YOUR "
"RECEIPT\t#\n#########################\n\n");

My next step was to re-format the program to render it more readable, and in
any case I've removed all the // comments, so the line numbers won't match
yours from now on.

foo.c:30: warning: initialization discards qualifiers from pointer target
type

Lots of these warnings, actually, and they are caused by:

char *p_menu_items[] = { "Bread",

I fixed this by using const char *, and I couldn't resist fixing the
spelling mistake at the same time:

const char *p_menu_items[] =
{
"Bread", "Butter", "Confectionery", "Fruit",
"Meat", "Milk", "Vegetables", "Other"
};

const char *p_menu_choices[] =
{
"Sub-total", "Total", "Quit"
};

Next was this:

foo.c:38: warning: function declaration isn't a prototype

I fixed this by changing:

int main()

to

int main(void)

foo.c:44: warning: nested extern declaration of `store_data'

I fixed this by shifting the declaration to file scope. Types at file scope,
objects at function scope. Types at file scope, objects at function scope.
Types at file scope, objects at function scope. Types at file scope,
objects at function scope. Types at file scope, objects at function scope.
Write it out ten times if need be - or a hundred times.

foo.c:71: parse error before `p_shopping'

strcpy(p_shopping.name, p_menu_items)
p_shopping.used = 1;

I checked your original code, and the same mistake is clearly there too - a
missing semicolon at the end of the strcpy call. No matter how lax your
compiler diagnostic settings, this is a show-stopping error.

You spoke of good programming habits. I would suggest the following list to
begin with:

1) use function scope for objects, not file scope;
2) avoid scanf until you're an expert; if your teacher requires its use,
plague him mercilessly with questions about it until it becomes apparent
that he doesn't understand it either;
3) if you must use // comments, make sure they're sufficiently short that
they are not broken into two or more lines when you post to Usenet;
4) use shorter functions - your main(), even shorn of comments, is around
140 lines long. Short, simple functions work best.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.

 

[Keith Thompson]

foo.c:30: warning: initialization discards qualifiers from pointer target
type

Lots of these warnings, actually, and they are caused by:

char *p_menu_items[] = { "Bread",

[...]

A technical point: This particular warning message is a bit
misleading. A warning is certainly appropriate here, but it should be
more specific.

The underlying problem is that string literals are not "const", but
attempting to modify a string literal invokes undefined behavior. gcc
has an option, "-Wwrite-strings", that causes it to warn about
attempts to modify string literals. But it does so by *pretending*
that string literals really are const. In this case, it's warning
that the "const" qualifier for the string literals is being discarded,
but in fact there is no such qualifier, either explicitly or
implicitly.

A compiler can warn about anything it likes, and the standard doesn't
require warnings to make sense, so this isn't a conformance issue.
And if you modify the code until all warnings disappear (which is
often a very good idea), this particular warning will have served its
purpose.

 

[Richard Heathfield]

Keith Thompson said:

foo.c:30: warning: initialization discards qualifiers from pointer target
type

Lots of these warnings, actually, and they are caused by:

char *p_menu_items[] = { "Bread",

[...]

A technical point: This particular warning message is a bit
misleading. A warning is certainly appropriate here, but it should be
more specific.

Yes, some of gcc's diagnostics leave much to be desired. Nevertheless, as
you rightly point out (in another part of your reply), this warning does
serve a purpose, however ineptly.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.

 

[Jens Thoms Toerring]

I don't ask anyone her to help me to cheat.

That's good to know;-) And my question wasn't meant as an insult but
just to remind you that asking her for help if you're not allowed to
is a stupid idea (one reason of many being that your lecturere could
very well be reading this group;-)

So my code follows:

<program code snipped>

Sorry, but as others already have told you, your program doesn't even
compile. In main() neither 'choice' nor 'sub_sum' are defined. It's
simple to add 'choice' but the value of 'sub_sum' gets used but never
set - obviously, you would you like that to be the value of the variable
with the same name defined in store_data(), but that's only visible
in that function. not in main().

And your program is messing around with memory in bad ways. To start
with, you allocate memory for one menu item less than you have (FIRST
is 7, but you have 8 menu items) and treat the memory you got as if
there would be enough. Then, in the infinite loop in main(), you
allocate memory for the menu strings each time round. This is, of
course, useless and you also lose the pointers to memory you already
got before, so you're unable to deallocate it - you're program is
"leaking" memory. Things only get worse in your function store_data(),
there are lots of off-by-one errors. No wonder you earlier or later
get a segmentation fault, it's all rather a mess.

I think that you should rewrite that program. And the rewrite should
include a complete redesign of the data structures. At the moment you
store "generic" goods like "Fruit" in the same kind of structure you
use for real goods, e.g. "Apples" or "Bananas". This doesn't make much
sense and makes it hard to understand your program (I am still not sure
what it's actually supposed to do...) as well as dealing with the
different kinds of things. Picking good data structures is essential
for writing good programs. Here's one way you could do it, using
different types of structures for the categories (basically equivalent
to your main menus entries) and the "real" goods the user buys:

typdef structure {
const char *name; /* name of "real" item bought */
double price; /* it's price */
} Goods_T;

typedef structure {
const char *name; /* name of category i.e. "Fruit" */
Goods_T *list; /* of bought goods belonging to this category */
int list_length; /* length of the array of bought goods */
double sub_total; /* total price of all goods in this category */
} Category_T;

You would start with allocating memory for as many category structures
as needed (8 at the moment). For each category you assign a name from the
p_menu_items array (no memory allocation needed - you already got memory
for these strings when you defined p_menu_items, so you just need to
assign a pointer to the relevant element of that array to the name member),
and you set 'list' to NULL, 'list_len' to 0 and 'total' to 0.0. Of course,
you can also start with simply defining an array of these structures and
initialize them in a single step, so you don't need malloc() at all for
the categories:

Category_T categories [ ] = { { "Bread", NULL, 0, 0.0 },
{ "Butter", NULL, 0, 0.0 },
... };

In that case you also don't need the p_mebu_items array anymore.

This out of the way you can ask the user what (s)he wants to buy. When
(s)he selects one of the menu items and then enters the name for some
"real" item you increase the length of the list of bought goods simply
by using realloc() on the 'list' member of the category structure the
item is supposed to go in - if you didn't know, you can use realloc()
also when you don't have any memory yet, if you call it with NULL as
the first argument it behaves like malloc(). Don't forget to increment
the list length variable. For the new list element you now allocate
memory for the name and the you set the price. You also add price to
the sub_total member of the category structure - that way you can keep
a sum for each of the categories, and when you have to print out the
final total you have to iterate over just the categories.

I guess that when you use data structures more suited to your problem
also writing the program will become simpler - you can have functions
that deal with categories and functions for individual items the user
buys and you don't have to do lots of checks what you're dealing with
at any moment. That, in turn, will make your program easier to under-
stand and thus to debug if there still are bugs.

With the new data structures spend some time writing functions. You
should have a function to add a new item to one of the categories, you
should have a function to print a category, using another function that
prints a single item etc. Have functions for the different types o
user input instead of mixing it all together. In the end, you will
have a very simple main() function that contains a loop and just a few
function calls. And each function will also be simple and easy to test.

Regards, Jens

PS:
// Neccessary to align, because there are numbers greater than 9
if ( i < 1 ) {
printf("\t\t\t %d. %s\n", ( i + 9 ), p_menu_choices);
}
else {
printf("\t\t\t%d. %s\n", ( i + 9 ), p_menu_choices);
}

can be replaced by

printf("\t\t\t%2d. %s\n", ( i + 9 ), p_menu_choices);

"%2d" means: use at least two places to print that int, if necessary
putting a space char in front.

 

[svata]

Consulting experts doesn't count as research?

Hopefully yes.

None of the experts here will help you to cheat, but we will help you to
learn, if you're willing for that to happen.

Yes, I'm willing for that.

You spoke of good programming habits. I would suggest the following list to
begin with:

1) use function scope for objects, not file scope;

I will do my best to try it out.

2) avoid scanf until you're an expert; if your teacher requires its use,
plague him mercilessly with questions about it until it becomes apparent
that he doesn't understand it either;

Can you explain me why should I refrain from using scanf()? Is that
function cursed or what? I see real difference in using gets() of
fgets(). But Borland C compiler gives no warning about using gets...

So, please, what I am supposed to use instead of scanf()?

3) if you must use // comments, make sure they're sufficiently short that
they are not broken into two or more lines when you post to Usenet;

Yes, this is a mistake... I will keep eye on that

4) use shorter functions - your main(), even shorn of comments, is around
140 lines long. Short, simple functions work best.

I know, but as everyone can imagine here, it distincts a good/skilled
programmer from a newbie. So I'm the newbie and I have to get used to
it.

But anyway, many thanks for your time.

svata

 

[svata]

You have been asked multiple times to stop top-posting, yet you
insist on so doing. I conclude you really do not care whether or
not anyone reads your posts. I shall not be doing so.

Sorry, only Jens asked me so far... and since that time I don't do it.

svata

 

[Richard Heathfield]

svata said:

Can you explain me why should I refrain from using scanf()? Is that
function cursed or what?

No, but it's very easy to misuse it. Most teachers teach it badly. Yours
does, for example.

I see real difference in using gets() of fgets().
Good.

But Borland C compiler gives no warning about using gets...

It isn't required. It's just a good idea.

So, please, what I am supposed to use instead of scanf()?

In my experience, the best way to capture input is as a string. Then parse
it yourself if you were hoping for numbers and stuff.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.

 

[Flash Gordon]

Richard Heathfield wrote:


Can you explain me why should I refrain from using scanf()? Is that
function cursed or what? I see real difference in using gets() of
fgets(). But Borland C compiler gives no warning about using gets...

OK, you are paying attention to warnings. That is a good thing. NEVER
use gets. Never. Not under any circumstances. The way gets works is that
you pass it a pointer to a buffer but it has absolutely no knowledge of
how long the buffer is. So you pass a pointer to a buffer 10 characters
long and the user enters 10 character and gets writes off the end of the
buffer stomping over some random piece of data. Well, in general the
piece of data stomped on is not random, and as a result of the I believe
people have in the past successfully exploited buffer overflows where
gets was used to do nasty things.

So, please, what I am supposed to use instead of scanf()?

fgets, fgetc or getc are generally good starting points. fgets and getc
both have gotchas that you have to be wary of but once you know them
they are easy to use correctly.

getc is a macro that is allowed to evaluate its argument more than once,
so doing getc(file_array[i++]) you would not know how many times I would
be incremented.

fgets leave the trailing newline character on the end of the data *if*
the buffer was large enough, otherwise it only reads in as much data as
will fit leaving room for the /0 to terminate the string, which it
always writes. So you have to check to see if the string has a newline
at the end to know if you read the complete line and if not decide how
to deal with only having read part of a line, one option being using
getc/fgetc to read and discard characters until you hit a new line or
end of file.

fgetc hasneither of these problems.

Yes, this is a mistake... I will keep eye on that

It is better not to use // comments at all for posting to news groups,
then the problem does not arrive and it also gives those with C90
compliant compilers a chance of trying the code without editing it.

I know, but as everyone can imagine here, it distincts a good/skilled
programmer from a newbie. So I'm the newbie and I have to get used to
it.

But anyway, many thanks for your time.

A newbie who is prepared and able to learn does not stay a newbie. So
learn, fix the problems that you can in your code, and post again to see
what else can be learned.