MALLOC problem

[sajjad]

Hello friends

I do not got Malloc to work

Please take a
look at this source code and tell me whats wrong. I have marked the 2
problem areas.

/* This program will sort an array of floats from the user.*/

#include <stdio.h>
#include <stdlib.h>
/* prints the contents of the array to stdout. array is the array
to print, size is it's size */
void prn_array(float *array, int size) {
int loopcount; /* used as a loop counter */
printf("\nThe sorted array is:\n");
for (loopcount=0; loopcount < size; loopcount++)
printf("%d ",array[loopcount]);
printf("\n");
}

/* swaps two floats from the array. num_one and num_two are the floats */
int swap( float *num_one, float *num_two) {

float temporary; /* temporary variable */
temporary = *num_one;
*num_one = *num_two;
*num_two = temporary;

}

/* sorts array in descending order. array is the array
to print, size is it's size */
void sort_descending (float array[], int size) {

int i, j; /* integers */

for (i=0; i<size - 1; ++i)
for (j= size-1; i<j;--j)
if (array[j-1] < array[j])
swap(&array[j-1], &array[j]);

}

/* sorts array in ascending order. array is the array
to print, size is it's size */
void sort_ascending (float array[], int size) {

int i, j; /* integers */

for (i=size; i>0 - 1; --i)
for (j= 0-1; i>j;++j)
if (array[j-1] > array[j])
swap(&array[j-1], &array[j]);

}

main() {

int size; /* number of floats in the array */
int *pointer, loopcount, choice; /* pointer is an int pointer,
loopcount is used as a loop counter, choice denotes users choice */
float array[size]; /* <---------------------- Problem one */

/* A welcome message to the user*/
printf("\nWelcome in this program
you will enter an array of floats and the program will sort it in
ascending
or descending order and and finally print it");
/* asking for the numbers of floats in the array */
printf("\nHow many numbers would you like sorted? ");
/* read user input and place into size variable */
scanf ("%d, &size");

array= (float *)malloc(size * sizeof(float)); /* <----------- Problem
two */

/* asking for the array */
/* branch based on size entered */
if(size==1)
printf("\nPlease enter 1 number");
else
printf("\nPlease enter %d numbers");

for (loopcount=0;loopcount<size; loopcount++)
scanf("%f", array[loopcount]);
while (choice==0){
printf("\n Please choose one of the following:");
printf("\n 1. Sort in ascending order ");
printf("\n 2. Sort in descending order");
printf("\n Enter choice --> ");
scanf ("%d", choice);
if ((choice==1)||(choice==2))
break;
}
/* which sort to use, branch on choice */
if(choice==1)
sort_ascending(array, size); /* sort the array in descending order */
else if (choice==2)
sort_descending(array, size); /* sort the array in descending order
*/

/* output the contents of the array */
prn_array(array, size);

}

Please help

 

[Seebs]

I do not got Malloc to work

Okay.

You know... Your subject line says "MALLOC". Here, you say "Malloc".

C is case sensitive. I urge you to put in the extra effort to be
consistent and correct with capitalization even when writing about C,
you'll find that it makes it easier to get programs right.

Please take a
look at this source code and tell me whats wrong. I have marked the 2
problem areas.

No, you haven't.

/* prints the contents of the array to stdout. array is the array
to print, size is it's size */
void prn_array(float *array, int size) {
int loopcount; /* used as a loop counter */
printf("\nThe sorted array is:\n");
for (loopcount=0; loopcount < size; loopcount++)
printf("%d ",array[loopcount]);

You did not mark this, but it is in fact a problem area, because you
are passing a float to printf, but giving it a %d format specifier
which expects an int. Try
printf("%f ", array[loopcount]);

/* sorts array in descending order. array is the array
to print, size is it's size */
void sort_descending (float array[], int size) {
int i, j; /* integers */

This comment is completely useless. Don't put in a comment if
it isn't going to give the reader any information.

for (i=0; i<size - 1; ++i)
for (j= size-1; i<j;--j)

Your spacing here is inconsistent. While this has no immediate effect
on generated code, it makes life harder for the reader. Try to be
consistent about spacing. For instance, in these two lines, you have
no spaces on either side of the '<', but on the next line, you
have spaces on both sides.

Carelessness about this kind of thing has two effects:
1. It makes the code harder to read.
2. It makes the reader distrust you.

if (array[j-1] < array[j])
swap(&array[j-1], &array[j]);

This is pretty inefficient.

main() {

You really ought to declare this properly as "int main(void)".

int size; /* number of floats in the array */

This is a good comment.

int *pointer, loopcount, choice; /* pointer is an int pointer,
loopcount is used as a loop counter, choice denotes users choice */

Again, the comment "is an int pointer" is TOTALLY USELESS. Don't give
meaningless comments.

float array[size]; /* <---------------------- Problem one */

Okay, here's your problem. What is "size" *right now*? Well, you haven't
initialized it, so it's garbage. You've just tried to declare an array
of unknown size.

In C89, you can't do this at all -- array sizes have to be constant.
In C99, you could do this if you had already set "size" to the right
value. But it's almost certainly the case that you don't mean to do
this at all, since you're talking about "malloc". So try:
float *array;

This will let you declare a pointer which can point to the storage you
allocate for your array.

/* read user input and place into size variable */
scanf ("%d, &size");
Okay.

array= (float *)malloc(size * sizeof(float)); /* <----------- Problem
two */

First off, drop the cast -- it's useless in C. Secondly, you're trying
to assign to an array. Arrays are not modifiable. You can modify their
elements, but not the array itself -- it's not a modifiable lvalue.

If you had declared a POINTER, rather than an array, this could work.

(If you are about to say something about how pointers and arrays are
the same thing, stop and go read the comp.lang.c FAQ section on pointers
and arrays.)

for (loopcount=0;loopcount<size; loopcount++)
scanf("%f", array[loopcount]);

Here, you have omitted the "&" you need. You can't pass the float to
scanf; you have to pass its ADDRESS to scanf, just as you did above when
you used scanf for size.

while (choice==0){
printf("\n Please choose one of the following:");
printf("\n 1. Sort in ascending order ");
printf("\n 2. Sort in descending order");
printf("\n Enter choice --> ");
scanf ("%d", choice);
if ((choice==1)||(choice==2))
break;
}

Consider what will happen if the user makes a mistake and enters "3" here.
Your loop terminates, but choice will not be 1 or 2.

/* which sort to use, branch on choice */
if(choice==1)
sort_ascending(array, size); /* sort the array in descending order */
else if (choice==2)
sort_descending(array, size); /* sort the array in descending order
*/

These comments, again, are pretty useless.

Please help

See above. The very shortest path to a fix will be the %d=>%f fix,
adding the & on array[loopcount], and fixing the declaration of array.
However, there's many other things you should improve.

Remember this: Sloppy work is habit-forming. Take every program you
write seriously, respect it, and do your best every time. If you do this,
you will find programming a great deal more fun than if you try to cut
corners on programs you don't think are important.

-s

 

[Morris Keesan]

On 2010-10-15, sajjad <[email protected]> wrote:

Okay.

Not okay.

 

[Seebs]

Not okay.

.... Woah. I can't believe I missed that.

Sajjad: This is why it's important to be good and consistent about your
spacing and layout -- it makes things like this easier to spot.

Basically, it's pretty hard for people to actually process all of the
details at once consciously (perhaps impossible for most of us), so we
rely heavily on our less-conscious mind's ability to pre-sort the
data it presents to us. Style conventions make that task much
easier.

-s

 

[luser- -droog]

... Woah.  I can't believe I missed that.

Sajjad:  This is why it's important to be good and consistent about your
spacing and layout -- it makes things like this easier to spot.

Basically, it's pretty hard for people to actually process all of the
details at once consciously (perhaps impossible for most of us), so we
rely heavily on our less-conscious mind's ability to pre-sort the
data it presents to us.  Style conventions make that task much
easier.

I'm relatively new at this, but I think you've been trolled.
You could have probably substituted FAQ citations for each
sentence you typed. In fact, he probably printed the FAQ TOC,
made confetti, and grabbed a handful of suggestions to consciously
violate for each line he typed.

I mean really: loopcounter /* the loop counter */
REALLY?

 

[Nobody]

I mean really: loopcounter /* the loop counter */
REALLY?

It's not that uncommon. Take a self-taught programmer who has never
bothered to comment their code, and put them on a programming course.
They'll be told how important it is to comment code, so they'll start
adding (often meaningless) comments to every declaration and statement.

 

[sajjad]

It's not that uncommon. Take a self-taught programmer who has never
bothered to comment their code, and put them on a programming course.
They'll be told how important it is to comment code, so they'll start
adding (often meaningless) comments to every declaration and statement.

Our course teaches good programming practise - EVERY variable must be
commented, also EVERY functions purpose explained and EVERY argument
commented, and only use meaningful names for functions and variables.

 

[Ian Collins]

Our course teaches good programming practise - EVERY variable must be
commented, also EVERY functions purpose explained and EVERY argument
commented, and only use meaningful names for functions and variables.

Pointless comments are a distraction, not good programming practise.

 

[Seebs]

Our course teaches good programming practise

No, it doesn't.

- EVERY variable must be
commented,

That's not really a particularly good practice.

Coupled with BAD comments, it is in fact an atrocious practice.

You are giving bad comments, which are much worse than no
comments at all.

also EVERY functions purpose explained and EVERY argument
commented, and only use meaningful names for functions and variables.

You don't really do any of these, though.

-s

 

[luser- -droog]

It's not that uncommon. Take a self-taught programmer who has never
bothered to comment their code, and put them on a programming course.
They'll be told how important it is to comment code, so they'll start
adding (often meaningless) comments to every declaration and statement.

But it does signal a gap in the hand-eye-brain circuit.
I mean ... I got bored typing it as a joke!
After hitting o for the sixth time on one line,
one should wonder, ever so slightly, if any of them
need to be there at all.

But this is the classic example, VERBATIM! isn't it?

 

[Felix Palmen]

int loopcount; /* used as a loop counter */

Oh, really?

float temporary; /* temporary variable */

You don't say

At least, state the /intention/ of the variable in the comment.

 

[Nick Keighley]

It's not that uncommon. Take a self-taught programmer who has never
bothered to comment their code, and put them on a programming course.
They'll be told how important it is to comment code, so they'll start
adding (often meaningless) comments to every declaration and statement.

I saw this once (well nearly this, I forget the atcual header name)

#include "crtwxe.h" /* obvious use */

 

[Nick Keighley]

Our course teaches good programming practise

perhaps you should follow them!



- EVERY variable must be

commented, also EVERY functions purpose explained

why? If a function is well named and uses sensible types it may be
perfectly obvious what it does

and EVERY argument
commented,

no. Really no.

void add_block_to_queue (Queue *q, const Block *b);

how does a comment help? (well ok, error handling)

and only use meaningful names for functions and variables.

"meaningful" doesn't mean loopcount is better than i

 

[August Karlstrom]

Our course teaches good programming practise - EVERY variable must be
commented, also EVERY functions purpose explained and EVERY argument
commented, and only use meaningful names for functions and variables.

If you have to, you have to. However, as others have mentioned, there is
no sense in adding comments that say the same thing as the code, like
for instance

/* output the contents of the array */
prn_array(array, size);

Do you think the above comment makes the statement easier to understand?

Good luck with your C programming.


/August

 

[Felix Palmen]

no. Really no.

Depends. In my opinion, it's good practice to document every argument as
well as the return value of a function IFF this function is part of a
public API. Also add exceptions for languages that have them.

And, of course, it should be done in a standard format that allows for
automatic generation of API documentation (e.g. visual-studio XML
comments, javadoc or doxygen).

Regards,
Felix

 

[Keith Thompson]

If you have to, you have to. However, as others have mentioned, there is
no sense in adding comments that say the same thing as the code, like
for instance

/* output the contents of the array */
prn_array(array, size);

Do you think the above comment makes the statement easier to understand?

Without the comment, I wouldn't have been at all sure that "prn" is an
abbreviation for "print".

If the function has the more sensible name "print_array", the comment
would be unnecessary.

 

[August Karlstrom]

Without the comment, I wouldn't have been at all sure that "prn" is an
abbreviation for "print".

If the function has the more sensible name "print_array", the comment
would be unnecessary.

Agreed, "only use meaningful names" as Sajjad says above.


/August

 

[Geoff]

Our course teaches good programming practise - EVERY variable must be
commented, also EVERY functions purpose explained and EVERY argument
commented, and only use meaningful names for functions and variables.

This is an attempt by your instructor to discern whether you
understand and can describe what you think you are doing in your code.
It's possible to write code that "works" but doesn't do what the
author thinks it does. To conform with the "house coding rules" in
your course work you must do this but you should write comments where
it is meaningful and in C you don't need to write that int i; is an
integer. It is more meaningful to state it is an outer loop counter or
some such.

When you are writing code to a specification the specification will
describe what the code is supposed to do. It's the coder's job to make
the code behave as described in the specification in a clear and
maintainable way. With "large" or complex functions this is essential.
With small functions like swap, it is often unnecessary and
distracting.

As for style, you should definitely use more white space in your code.
Your example is dense and hard to read. Conform to the known standards
style and stick with them. You should also work to make your functions
more generic. Your prn_array is not really generic. If you wrote:

/* prints the contents of the array to stdout.
array is the array to print, size is it's size */
void print_array(float *farray, int size)
{
int counter;

for (counter = 0; counter < size; counter++)
printf("%f ", farray[counter]);
printf("\n");
}

.... then you can write:

/* output the contents of the array */
printf("\nThe sorted array is:\n");
print_array(farray, size);
return 0;
}

in main and you can write

print_array(farray, size);

anywhere in your program to debug the state of the array at any time.

You should also try to write your array indexing as though you know
what you are doing and know how to maintain bounds. Don't write silly
things like this:

void sort_descending (float array[], int size) {

int i, j; /* integers */

for (i=0; i<size - 1; ++i)
for (j= size-1; i<j;--j)
if (array[j-1] < array[j])
swap(&array[j-1], &array[j]);

}

/* sorts array in ascending order. array is the array
to print, size is it's size */
void sort_ascending (float array[], int size) {

int i, j; /* integers */ /* no kidding? */

for (i=size; i>0 - 1; --i)
for (j= 0-1; i>j;++j) /* 0-1? what the heck??? */
if (array[j-1] > array[j])
swap(&array[j-1], &array[j]);

}

when you should be writing something like this:

/* sorts array in descending order. array is the array
to print, size is it's size */
void sort_descending (float farray[], int size)
{
int i; /* outer loop counter */
int j; /* inner loop counter */

for (i = size; i > 0; --i)
for (j = 1; i > j; ++j)
if (farray[j-1] < farray[j])
swap(&farray[j-1], &farray[j]);
}

/* sorts array in ascending order. array is the array
to print, size is it's size */
void sort_ascending (float farray[], int size)
{
int i;
int j;

for (i = size; i > 0; i--)
for (j = 0; i > j; j++)
if (farray[j-1] > farray[j])
swap(&farray[j-1], &farray[j]);
}

It is very clear your problem will malloc prevented you from debugging
these functions.

 

[John Bode]

Our course teaches good programming practise - EVERY variable must be
commented, also EVERY functions purpose explained and EVERY argument
commented, and only use meaningful names for functions and variables.

Comments on variable declarations are only useful if a) they are
correct, and b) they give information not already present in the
variable name. The excerpt cited by luser- -droog is an example of a
poor comment; it doesn't tell us anything we don't already know about
that variable. At best, it's redundant and adds visual clutter.

There's a point where commenting for the sake of commenting makes the
code *harder* to read and maintain. When writing a comment, ask
yourself if you're providing useful information that's not apparent in
the code itself. If the answer is "no", then don't add the comment.

 

[Keith Thompson]

Comments on variable declarations are only useful if a) they are
correct, and b) they give information not already present in the
variable name. The excerpt cited by luser- -droog is an example of a
poor comment; it doesn't tell us anything we don't already know about
that variable. At best, it's redundant and adds visual clutter.

There's a point where commenting for the sake of commenting makes the
code *harder* to read and maintain. When writing a comment, ask
yourself if you're providing useful information that's not apparent in
the code itself. If the answer is "no", then don't add the comment.

Agreed, mostly.

An exception might be in an assignment for a beginning programmer
(either learning the language or just learning to program), when
the purpose of a comment isn't so much to convey information to
the reader, but to demonstrate that the writer knows what he's doing.

The question is, how does the student break the habit of writing
elementary comments as he progresses? I don't have a good answer to
that.