First Program

[mwt]

Hello. Today I wrote my first program in C. It adds up the elements in
an array. I am just beginning to learn this language. Any tips or
pointers about better ways to write/structure/format/etc. this code
would be much appreciated. Thanks.
mwt.

#include <stdio.h>

int add_array(int arr[], int arr_size)
{
int j;
int total = 0;
for(j = 0; j < arr_size; j++)
total += arr[j];
return(total);

}

main()
{
int size;

printf("Input size of array>");
scanf("%d", &size);
int array[size];
int i;
for(i=0; i<size; i++)
{
array = i+1;

}

printf("Sum of array elements is %d.\n", add_array(array,
size));
}

 

[osmium]

Hello. Today I wrote my first program in C. It adds up the elements in
an array. I am just beginning to learn this language. Any tips or
pointers about better ways to write/structure/format/etc. this code
would be much appreciated. Thanks.
mwt.

#include <stdio.h>

int add_array(int arr[], int arr_size)
{
int j;
int total = 0;
for(j = 0; j < arr_size; j++)
total += arr[j];
return(total);

}

main()
{
int size;

printf("Input size of array>");
scanf("%d", &size);
int array[size];
int i;
for(i=0; i<size; i++)
{
array = i+1;

}

printf("Sum of array elements is %d.\n", add_array(array,
size));
}

Assuming that it works, I would say you are off to a good start.

 

[Burton Samograd]

Hello. Today I wrote my first program in C. It adds up the elements in
an array. I am just beginning to learn this language. Any tips or
pointers about better ways to write/structure/format/etc. this code
would be much appreciated. Thanks.

#include <stdio.h>

int add_array(int arr[], int arr_size)
{
int j;
int total = 0;
for(j = 0; j < arr_size; j++)
total += arr[j];
return(total);

}

main()
{
int size;

printf("Input size of array>");
scanf("%d", &size);
int array[size];

^--- looks like a gcc feature so it might not be portable
(is this in C99?).

you should use:

int *array = malloc(size*sizeof(*array))

int i;
for(i=0; i<size; i++)
{
array = i+1;
}

printf("Sum of array elements is %d.\n", add_array(array,
size));

and don't forget to free it when you're finished with it:

free(array)
}
Other than that it looks good. Style is fine AFAICT.

 

[mwt]

^--- looks like a gcc feature so it might not be portable
(is this in C99?).

I am compiling with gcc. Other than that, I have no idea what type of C
it is.
For learning maximally portable features, what linux-based compiler (or
perhaps flags on gcc) should I be using?

 

[mwt]

Ah. I see. If I use the -pendantic option in gcc, it enforces strict
compliance with ISO C90, and won't allow the

int array[size];

declaration.
Interesting.

 

[Default User]

main()
{
int size;

printf("Input size of array>");
scanf("%d", &size);
int array[size];

Assuming that it works, I would say you are off to a good start.

You're pretty easy. What version of C allows both implicit declaration
of functions AND variable-length arrays?




Brian

 

[Roberto Waltman]

Hello. Today I wrote my first program in C. It adds up the elements in
an array. I am just beginning to learn this language. Any tips or
pointers about better ways to write/structure/format/etc. this code
would be much appreciated. Thanks.
mwt.

Compiling (under Linux) with "gcc -Wall -ansi -pedantic first.c"
produces a few warnings. I inserted them at the matching line.

#include <stdio.h>

int add_array(int arr[], int arr_size)
{
int j;
int total = 0;
for(j = 0; j < arr_size; j++)
total += arr[j];
return(total);

( The parenthesis are not necessary. I tend to use them here myself)

}

main()

first.c:14: warning: return type defaults to `int'
(Should be "int main(void)" if you do not process command line
parameters)

{
int size;

printf("Input size of array>");
scanf("%d", &size);
int array[size];

first.c:19: warning: ISO C89 forbids variable-size array `array'
first.c:19: warning: ISO C89 forbids mixed declarations and code
(Probably too early for you to bother about different versions of C,
but be aware that the language has changed over time)

int i;
for(i=0; i<size; i++)
{
array = i+1;

}

printf("Sum of array elements is %d.\n", add_array(array,
size));

first.c:29: warning: control reaches end of non-void function
( missing return EXIT_SUCCESS; - defined in stdlib.h }

}

A few comments on style: be consistent. You use braces around the
single statement in the for() loop in the main function, (which is
good,) but not in a similar loop in add_array().

Likewise, you use a "double level" indentation for the function
blocks, but not for the for() loops. Pick a style and stick with it.

A few comments describing the program's purpose would be helpful. They
become much more important for more complex programs.

All in all, very good for a first program!

 

[Default User]

Hello. Today I wrote my first program in C. It adds up the elements in
an array. I am just beginning to learn this language. Any tips or
pointers about better ways to write/structure/format/etc. this code
would be much appreciated. Thanks.
mwt.

#include <stdio.h>

int add_array(int arr[], int arr_size)
{
int j;
int total = 0;
for(j = 0; j < arr_size; j++)
total += arr[j];
return(total);

The parentheses around total are necessary. While not harmful, it
implies that you believe return to be function.

}

main()

This should be declared as:

int main(void)

{
int size;

printf("Input size of array>");
scanf("%d", &size);

You have no way of know whether the person entered a legal value for
size here. Hint, scanf() returns a value, find out why and what it
means. You should also use an unsigned type for size, as you don't want
people entering a negative number. At the very least, you need to check
the number to make sure it's greater than 0.

int array[size];
int i;
for(i=0; i<size; i++)
{
array = i+1;

}

printf("Sum of array elements is %d.\n", add_array(array,
size));
}

The things used above (variable length arrays and declaration of
objects following code) are C99 features. That's not to say that you
shouldn't learn them, just be aware that you won't find them in some
textbooks, notably K&R.

If you are going to use C99, I would recommend moving the declaration
of i into the for loop:

for (int i=0; i<size; i++)


While it is ok to leave out a return from main() in C99, I prefer to
see an explicit one anyway. Purely stylistic.




Brian

 

[Keith Thompson]

Hello. Today I wrote my first program in C. It adds up the elements in
an array. I am just beginning to learn this language. Any tips or
pointers about better ways to write/structure/format/etc. this code
would be much appreciated. Thanks.
mwt.

If this is your first C program, it's an excellent start. I'll offer
a few minor suggestions.

#include <stdio.h>

int add_array(int arr[], int arr_size)

Keep in mind that, in a parameter declaration "int arr[]" is exactly
equivalent to "int *arr"; in other words, you're really passing a
pointer, not an array. Section 6 of the comp.lang.c FAQ,
<http://www.c-faq.com/>, covers arrays and pointers very well.

Some programmers (myself included) prefer not to use this
looks-like-an-array-but-acts-like-a-pointer declaration, but that's
just a matter of style; feel free to use whatever you're comfortable
with.

{
int j;
int total = 0;
for(j = 0; j < arr_size; j++)
total += arr[j];
return(total);

The parentheses are unnecessary. Again, this is a matter of style,
but I prefer "return total;"; a return statement isn't a function
call, and it shouldn't look like one.

}

main()

This should be "int main(void)".

{
int size;

Consider calling this "length" rather than "size". The word "size"
implies sizeof, which gives the size of an object in bytes; the number
of elements in an array is its length. (If sizeof(int)==4, a
10-element array of int has a length of 10 and a size of 40.)

printf("Input size of array>");

Due to buffering, it's not guaranteed that this will appear
immediately. Add "fflush(stdout);" to ensure that the prompt appears
before the program starts waiting for input.

scanf("%d", &size);

You'll find that the scanf() function has some pitfalls. In this
case, it will skip all leading whitespace, including newlines. If you
enter a valid integer, it will leave any characters *following* that
to be read by further calls; for example, if you type "123<ENTER>",
then read a single character, the character you read will be the
new-line character.

It doesn't matter in a small program like this, but it will matter
later on. One good approach is to use fgets() (*never* use gets()) to
read a full line, then use sscanf() to parse it.

Also, scanf() returns a result, which you're ignoring. What happens
if you enter something other than a valid number?

int array[size];

There are two C standards, C90 and C99. C99 theoretically supersedes
C90, but it's not yet universally implemented. For maximum
portability, you should consider sticking to C90 features.

Here you're using two C99-specific features (or perhaps gcc
extensions). C90 doesn't allow declarations to follow statements
within a block, though you can work around it by introducing a new
block. Also, C90 requires array lengths to be compile-time constants;
C99 introduces variable-length arrays, and gcc has supported something
very similar as an extension for some time. Burton Samograd already
suggested using malloc() instead (which also has the advantage that
you can catch allocation errors).

int i;
for(i=0; i<size; i++)
{
array = i+1;

}

printf("Sum of array elements is %d.\n", add_array(array,
size));

You should add "return 0;" here. (It's not strictly required in C99,
but it's a good idea anyway.

}

<OT>

To get warnings about non-standard code (i.e., to persuade gcc to act
as a conforming C compiler), you can use:
gcc -ansi -pedantic -Wall -Wextra
or, for C99:
gcc -std=c99 -pedantic -Wall -Wextra
(but see <http://gcc.gnu.org/c99status.html>).

The "-Wextra" option, equivalent to "-W", may give you more warnings
that are really appropriate.

</OT>

 

[Keith Thompson]

Ah. I see. If I use the -pendantic option in gcc, it enforces strict
compliance with ISO C90, and won't allow the

int array[size];

declaration.
Interesting.

Please read <http://cfaj.freeshell.org/google/>.

 

[mwt]

Please read <http://cfaj.freeshell.org/google/>.

I see. Yes, I'm using google and thinking of this as a web forum.
Whoops.

Thanks to everyone for all the pointers about my code. It's a little
confusing (what with differences of opinion, etc.), but I think I'm
getting it. So far, I have reworked the code to try and incorporate
some of these suggestions, although I have not yet created all the
proper error-checking features. I'll add those as soon as I study up
enough to understand what you're recommending .

Here's the current, semi-reworked version:

#include <stdio.h>
/*written 13Apr06 by mwt
This program is intended to give the sum of the integers in an array of
length n,.
where n is input by the user.*/

int add_array(int arr[], int arr_size)
{
int j;
int total = 0;
for(j = 0; j < arr_size; j++)
{
total += arr[j];
}
return total;

}

int main(void)
{
int size;
printf("Input size of array>");
scanf("%d", &size);
int *array = malloc(size*sizeof(*array));
int i;
for(i=0; i<size; i++)
{
array = i+1;
}

printf("Sum of array elements is %d.\n", add_array(array,
size));
free(array);
return 0;
}

It runs, but there is still some weirdness.

With my gcc compiler set with the flags -Wall -ansi -pedantic, it
complains about the "malloc" line in several ways:

add.c:23: warning: implicit declaration of function 'malloc'
add.c:23: warning: incompatible implicit declaration of built-in
function 'malloc'
add.c:23: warning: ISO C90 forbids mixed declarations and code

As well as the "free" line:

add.c:31: warning: implicit declaration of function 'free'

 

[Fred Kleinschmidt]

Hello. Today I wrote my first program in C. It adds up the elements in
an array. I am just beginning to learn this language. Any tips or
pointers about better ways to write/structure/format/etc. this code
would be much appreciated. Thanks.
mwt.

Compiling (under Linux) with "gcc -Wall -ansi -pedantic first.c"
produces a few warnings. I inserted them at the matching line.

#include <stdio.h>

int add_array(int arr[], int arr_size)
{
int j;
int total = 0;
for(j = 0; j < arr_size; j++)
total += arr[j];
return(total);

( The parenthesis are not necessary. I tend to use them here myself)

}

main()

first.c:14: warning: return type defaults to `int'
(Should be "int main(void)" if you do not process command line
parameters)

{
int size;

printf("Input size of array>");
scanf("%d", &size);
int array[size];

first.c:19: warning: ISO C89 forbids variable-size array `array'
first.c:19: warning: ISO C89 forbids mixed declarations and code
(Probably too early for you to bother about different versions of C,
but be aware that the language has changed over time)

int i;
for(i=0; i<size; i++)
{
array = i+1;

}

printf("Sum of array elements is %d.\n", add_array(array,
size));

first.c:29: warning: control reaches end of non-void function
( missing return EXIT_SUCCESS; - defined in stdlib.h }

}

A few comments on style: be consistent. You use braces around the
single statement in the for() loop in the main function, (which is
good,) but not in a similar loop in add_array().

Likewise, you use a "double level" indentation for the function
blocks, but not for the for() loops. Pick a style and stick with it.

A few comments describing the program's purpose would be helpful. They
become much more important for more complex programs.

All in all, very good for a first program!

Agreed!
Now the OP might want to consider these two variations of his program:
1) Find the mean value (sum divided by size) of the items in the array.
2) What might happen if the array is real numbers instead of integers?

In (1), obviously the mean is unlikely to be an exact integer, so the answer
should be stated as a double. So his summation function, to become a
mean() method, might looks like:
double compute_mean( int *array, int size );
or for (2):
double compute_mean( double *array, int size );

However, adding up the numbers and then dividing by size could cause a
couple
of problems.
The obvious one is if size=0. OP: how would you handle this?
Another one is if there are n values, all greater than MAX_INT/n. OP: how
to handle this?
Another is if there are millions of values, and some are very small and
some are very large (i.e., LARGE_VALUE > SMALL_VALUE*10**16) OP: How to
handle this?

 

[Burton Samograd]

With my gcc compiler set with the flags -Wall -ansi -pedantic, it
complains about the "malloc" line in several ways:

add.c:23: warning: implicit declaration of function 'malloc'
add.c:23: warning: incompatible implicit declaration of built-in
function 'malloc'
add.c:23: warning: ISO C90 forbids mixed declarations and code

As well as the "free" line:

add.c:31: warning: implicit declaration of function 'free'

You have to include the prototypes for the malloc and free which are
found in stdlib.h, so put the line:

#include <stdlib.h>

at the beginning of your program to get rid of those warnings.

 

[Burton Samograd]

main()
{
int size;

printf("Input size of array>");
scanf("%d", &size);
int array[size];

Assuming that it works, I would say you are off to a good start.

You're pretty easy. What version of C allows both implicit declaration
of functions AND variable-length arrays?

gcc

 

[Keith Thompson]

Thanks to everyone for all the pointers about my code. It's a little
confusing (what with differences of opinion, etc.), but I think I'm
getting it. So far, I have reworked the code to try and incorporate
some of these suggestions, although I have not yet created all the
proper error-checking features. I'll add those as soon as I study up
enough to understand what you're recommending .

Here's the current, semi-reworked version:
[snip]
It runs, but there is still some weirdness.

With my gcc compiler set with the flags -Wall -ansi -pedantic, it
complains about the "malloc" line in several ways:

add.c:23: warning: implicit declaration of function 'malloc'
add.c:23: warning: incompatible implicit declaration of built-in
function 'malloc'
add.c:23: warning: ISO C90 forbids mixed declarations and code

As well as the "free" line:

add.c:31: warning: implicit declaration of function 'free'

As Burton Samograd mentioned, "#include <stdlib.h>" will fix the
warnings about malloc and free.

If you want C90 compatibility, you still need to avoid mixing
declarations and statements. For example, you have something like
(code mercilessly abbreviated to illustrate the point):

{
int size;
scanf("%d", &size);
int *array = malloc(size * sizeof *array);
...
}

Within the block, you have a delaration, a statement, and a
declaration, which is illegal in C90. (It's allowed in C99 <OT>and in
C++</OT>, and probably in GNU C.)

(OT is an abbreviation for "off-topic", since we don't discuss C++
here.)

There are (at least) two ways to fix this (other than using a C99
compiler and not worrying so much about portability):

1. Move all your declarations to the top of the block. If you have
any initializations that depend on preceding statements, change
them to assignments:

{
int size;
int *array;

scanf("%d", &size);
array = malloc(size * sizeof *array);
...
}

2. Create a nested block (a block is a statement):

{
int size;

scanf("%d", &size);
{
int *array = malloc(size * sizeof *array);
...
}
}

 

[mwt]

Thanks to everyone for all the pointers about my code. It's a little
confusing (what with differences of opinion, etc.), but I think I'm
getting it. So far, I have reworked the code to try and incorporate
some of these suggestions, although I have not yet created all the
proper error-checking features. I'll add those as soon as I study up
enough to understand what you're recommending .

Here's the current, semi-reworked version:
[snip]
It runs, but there is still some weirdness.

With my gcc compiler set with the flags -Wall -ansi -pedantic, it
complains about the "malloc" line in several ways:

add.c:23: warning: implicit declaration of function 'malloc'
add.c:23: warning: incompatible implicit declaration of built-in
function 'malloc'
add.c:23: warning: ISO C90 forbids mixed declarations and code

As well as the "free" line:

add.c:31: warning: implicit declaration of function 'free'

As Burton Samograd mentioned, "#include <stdlib.h>" will fix the
warnings about malloc and free.

If you want C90 compatibility, you still need to avoid mixing
declarations and statements. For example, you have something like
(code mercilessly abbreviated to illustrate the point):

{
int size;
scanf("%d", &size);
int *array = malloc(size * sizeof *array);
...
}

Within the block, you have a delaration, a statement, and a
declaration, which is illegal in C90. (It's allowed in C99 <OT>and in
C++</OT>, and probably in GNU C.)

(OT is an abbreviation for "off-topic", since we don't discuss C++
here.)

There are (at least) two ways to fix this (other than using a C99
compiler and not worrying so much about portability):

1. Move all your declarations to the top of the block. If you have
any initializations that depend on preceding statements, change
them to assignments:

{
int size;
int *array;

scanf("%d", &size);
array = malloc(size * sizeof *array);
...
}

2. Create a nested block (a block is a statement):

{
int size;

scanf("%d", &size);
{
int *array = malloc(size * sizeof *array);
...
}
}

Keith -
I've tried to incorporate your suggestions. I have looked up and
attempted to understand the fgets and sscanf functions you recommended.
Here is my modified main function. It does compile without errors and
run. Is this closer to what you're describing?

int main(void)
{
int length;
char c[4];
int *array;
int i;

printf("Input length of array (max. 4 digits)>");
fflush(stdout);
fgets(c, 4, stdin);
sscanf(c, "%d", &length);
array = malloc(length*sizeof(*array));

for(i=0; i<length; i++)
{
array = i+1;
}

printf("Sum of array elements is %d.\n", add_array(array,
length));
free(array);
return 0;
}

 

[Default User]

gcc

That's no version of C I've ever heard of. Now, it may be the case it's
a C compiler with platform-specific, non-standard extensions.




Brian

 

[Keith Thompson]

Keith -
I've tried to incorporate your suggestions. I have looked up and
attempted to understand the fgets and sscanf functions you recommended.
Here is my modified main function. It does compile without errors and
run. Is this closer to what you're describing?

Closer.

(BTW, don't quote the entire article to which you're replying; snip
out the parts that aren't relevant to your followup. In particular,
don't quote the signature unless you're comenting on it.)

int main(void)
{
int length;
char c[4];
int *array;

The name "array" is potentially misleading. The variable points to an
array; it itself is a pointer.

int i;

printf("Input length of array (max. 4 digits)>");
fflush(stdout);
fgets(c, 4, stdin);

I'd make the array bigger. Remember, you're trying to read an entire
line, and the string needs to hold the trailing new-line characters as
well.

Also, 4 is a "magic number". Use a macro to define the size of
your array:

#define MAX_LEN 100
char c[MAX_LEN];
...
fgets(c, MAX_LEN, stdin);
or even:
fgets(c, sizeof c, stdin);

(The latter fails if c is a pointer to an array rather than an actual
array.0

sscanf(c, "%d", &length);

What happens if the user enters "xyz"? sscan() returns a result; use
it.

array = malloc(length*sizeof(*array));

What happens if the user enters "0"? Or "-123"?

for(i=0; i<length; i++)
{
array = i+1;
}

printf("Sum of array elements is %d.\n", add_array(array,
length));

Long lines can be a problem, especially when posting to Usenet, where
software can split lines at arbitrary points. I'd write this as:

printf("Sum of array elements is %d.\n",
add_array(array, length));

though opinions vary considerably on how to indent continuation lines
like this.

 

[August Karlstrom]

Hello. Today I wrote my first program in C. It adds up the elements in
an array. I am just beginning to learn this language. Any tips or
pointers about better ways to write/structure/format/etc. this code
would be much appreciated. Thanks.
mwt.

#include <stdio.h>

int add_array(int arr[], int arr_size)

Hint: Which one of the statements

x = add_array(a, length);

and

x = array_sum(a, length);

do you think makes most sense?

When choosing a name of a subroutine that returns a value and has no
side effect, try to concentrate on the returned value. What is
returned?.. the array sum. (You may also want to look up the
command-query principle.)


August

 

[mwt]

The name "array" is potentially misleading. The variable points to an
array; it itself is a pointer.

int i;

printf("Input length of array (max. 4 digits)>");
fflush(stdout);
fgets(c, 4, stdin);

I'd make the array bigger. Remember, you're trying to read an entire
line, and the string needs to hold the trailing new-line characters as
well.

Also, 4 is a "magic number". Use a macro to define the size of
your array:

#define MAX_LEN 100
char c[MAX_LEN];
...
fgets(c, MAX_LEN, stdin);
or even:
fgets(c, sizeof c, stdin);

(The latter fails if c is a pointer to an array rather than an actual
array.0

sscanf(c, "%d", &length);

What happens if the user enters "xyz"? sscan() returns a result; use
it.

array = malloc(length*sizeof(*array));

What happens if the user enters "0"? Or "-123"?

I think the new code I wrote addresses these issues. Now, if the user
inputs non-digits, the program exits (not the best way to handle
things, but at least it works). I think that "0" is a valid digit to
input in this case, so I allow it. Certainly negative numbers would be
interesting to be able to process properly, although I'm not sure how
to go about it.

x = array_sum(a, length);

When choosing a name of a subroutine that returns a value and has no
side effect, try to concentrate on the returned value. What is
returned?.. the array sum. (You may also want to look up the
command-query principle.)

That makes sense. I've changed the function name to suit.

Here's the current iteration of this exercise. Thanks, again, to
everyone for all the input. Including the stuff on how to properly post
to the group.

#include <stdio.h>
#include <stdlib.h>
/* written 13Apr06 by mwt
*
* This program is intended to give the sum
* of the integers in an array of length n,.
* where n is input by the user.
*/

#define MAX_LEN 100

int array_sum(int *arr, int arr_length)
{
int j;
int total = 0;
for(j = 0; j < arr_length; j++)
{
total += arr[j];
}
return total;

}

int main(void)
{
int length;
char c[MAX_LEN];
int *int_array;
int i;
int sc;

printf("Input length of array>");
fflush(stdout);
fgets(c, MAX_LEN, stdin);
sc = sscanf(c, "%d", &length);
if(sc > 0 && length >=0)
{
int_array = malloc(length*sizeof(*int_array));

for(i=0; i<length; i++)
{
int_array = i+1;
}

printf("Sum of array elements is %d.\n",
array_sum(int_array, length));
free(int_array);
}
else
{
printf("Invalid input.\n");
}
return 0;
}