Re: C code is not generating required results.

Groovy hepcat vasudevmukherjee@xxxxxxxxxxx was jivin' on 14 Dec 2005
03:20:14 -0800 in comp.lang.c.
C code is not generating required results.'s a cool scene! Dig it!

>Hi! Can somebody help tell me why the following code gives a garbage
>value while producing first student's name, whereas it gives the names
>correctly for other three students - I really fail to understand since
>it is not generating the required results.- Thank you in anticipation -
>Vasudev.

Your code is extremely hard to read. This is due to the total lack
of indentation and other white space. You also have line wrapped
string literals. It's a mess! So, I have reformatted it. Now we can
begin to identify things that need attention.

>#include <stdio.h>
>
>main ()

Make that

int main(void)
>{
> char student[4][30], *x[4][30];
> int i, j, k;
>
> printf("Student's name!\n");
>
> for(i = 0; i <= 3; i++)

Don't use "magic numbers". You should use the actual number of
elements in the array. How do you do that? Simple! Divide the size of
the array by the size of an element. Or, to put it another way:

for(i = 0; i < (sizeof student / sizeof *student); i++)

> {
> gets(student[i]);

As other respondants have already told you, never use gets(). It
bears repeating. Never use gets(). Why? Simple! There is no way to
prevent the user of your program from entering more characters than
the array you've provided can hold. You have enough space for 29
characters (plus the terminating '\0'). Now, what if the user, either
through malice or sheer idiocy, enters 30 or more? It will overflow
your array, causing undefined behaviour; that's what.
So never use gets(). So what do you do instead? Again, simple! You
use fgets(). Look that one up in your C book. But first, read the FAQ
(http://www.eskimo.com/~scs/C-faq/top.html)

> x[i][30] = &student[i][30];

What on Earth is the point of that? There's absolutely no need. You
can use student[i] everywhere you use *x[i] below. So you just don't
need x at all.

> }
>
> printf("\nYou have typed the following names.\n");
>
> for(i = 0; i <= 3; i++)
> {
> puts(*x[i]);
> }
>
> printf("\nHow many names do you want to republish (choice 1 to 4)??\n");
> scanf("%d", &k);

When you read the FAQ you will see that scanf() is not the best
function to use for this and what to do instead.

> k = k - 1;

What's the point of that? The usual form of a for loop is for(X = 0;
X < Y; X++) where X is the loop index and Y is the number of times you
want to loop. So, in this case, you want k to be the number of times
you want to loop, not one less than that number.

> if(k >= 0)

What about the possibility of k being greater than 3 (or 4 if you do
the loop the usual way as shown above)? A better approach would be to
write a function to get a number and make sure it is within range.
(See my code below.)

> {
> for(j = 0; j <= k; j++)

You were using i for your loop index before. Why change it now? Be
consistent. This is the sort of thing that can make code harder to
understand. Think about the next person who has to read your code
(whether it be yourself or someone else), and do what he/she most
expects.

> {
> printf("Which Student's name do you want to know, 1st, 2nd, 3rd or 4th??\n");
> scanf("%d", &i);
> i = i - 1;

Or, in other words:

i--;

> printf("Student's name = %s\n", *x[i]);
> }
> }

Return something. Portable return values for main() are 0,
EXIT_SUCCESS and EXIT_FAILURE (the latter two being macros defined in
stdlib.h).

return 0;
>}

Here's an improved version of your code using extra functions to get
input.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

#define ARRAY_SIZE(a) (sizeof a / sizeof *a)

/* Read in a line of text from stdin. Read only up to
enough characters to fit into buf, discarding the
rest of the line if more characters remain. Remove
newline from the input. */
void get_line(char *buf, size_t buflen)
{
char *p;

if(fgets(buf, buflen, stdin))
{
/* Find the newline (which fgets() retains)... */
p = strchr(buf, '\n');
if(p)
{
/* ...and remove it. */
*p = '\0';
}
else
{
/* Newline not found: more input remains in stdin. Remove it. */
int c;

while(EOF != (c = getchar()) && '\n' != c)
;
}
}
else
{
fputs("\nError reading from stdin.\n", stderr);
exit(EXIT_FAILURE);
}
}

/* Display a prompt and read in a number within a certain range.
Keep trying until a number (and ONLY a number) is read in
and it is within the given range. */
int get_num_in_range(const char *prompt, int low, int high)
{
char buf[20];
int n;
char c;

if(low > high)
{
/* Swap 'em. */
n = low;
low = high;
high = n;
}

do
{
/* Display the prompt. */
printf("%s (%d to %d): ", prompt, low, high);
fflush(stdout);

/* Get the input. */
get_line(buf, sizeof buf);

/* Extract the number. Loop if not (only) a number
or if number not in range. */
}while(1 != sscanf(buf, "%d%c", &n, &c) || n < low || n > high);

return n;
}

int main(void)
{
char student[4][30];
int i, j, k;

printf("Student's name:\n");
fflush(stdout);

for(i = 0; i < ARRAY_SIZE(student); i++)
{
/* Get the name. */
get_line(student[i], sizeof student[i]);
}

printf("\nYou have typed the following names.\n");

for(i = 0; i < ARRAY_SIZE(student); i++)
{
puts(student[i]);
}

k = get_num_in_range("\nHow many names do you want to republish?",
1, ARRAY_SIZE(student));

for(i = 0; i < k; i++)
{
j = get_num_in_range("Which Student's name do you want to know?",
1, ARRAY_SIZE(student));
j--;

printf("Student's name = %s\n", student[j]);
}

return 0;
}

--

Dig the even newer still, yet more improved, sig!

http://alphalink.com.au/~phaywood/
"Ain't I'm a dog?" - Ronny Self, Ain't I'm a Dog, written by G. Sherry & W. Walker.
I know it's not "technically correct" English; but since when was rock & roll "technically correct"?
.

Relevant Pages

  • Re: Comparing pointers to NULL
    ... Suppose our data is an array of records with each record having many ... In C it is natural to use a struct to represent a record and an ... in the body of a loop over record number the current record number ...
    (comp.lang.c)
  • Re: Streaming files
    ... int lIndex = 0; ... Why are you opening and closing the file for each iteration of the loop? ... As you are increasing lIndex for every iteration in the loop, ... If you want to fit all that data into the array, ...
    (microsoft.public.dotnet.languages.csharp)
  • Re: Help a beginner - simple lowercase to uppercase and so on function
    ... And then one to loop across the string calling that function ... copying at to a new array. ... are any characters other than lowercase letters */ ...
    (comp.lang.c)
  • Re: read keyboard input and storing in an array?
    ... When in a loop, how do I simply ... The data returned by 'br.readLine' would always be a String so I've stored ... simply convert it to an int as follows: ... you could copy 'myInt' to your array. ...
    (comp.lang.java.help)
  • Re: gets and scanf
    ... but "int main" is better. ... The getsfunction reads characters from stdin, ... successively in the array you specified. ... reaches a newline. ...
    (comp.lang.c.moderated)