# Thread: What is wrong with this code?

1. Hardcore Techie Join Date
Jul 2002
Posts
1,391
Rep Power
0

## What is wrong with this code?

Please give an open critique of this code?

http://pastebin.com/yhQ0QaXy  Reply With Quote

2. ## when you assign the arrays to the pointers you dont need the []
the function calls dont need data types on them.
you are returning values without storing them. plus_total = total ; and minus_total = total; cant see the "total" inside your sum() function.

should be:
Code:
```minus_total =  sum(a,array_amount);

plus_total = sum(a,array_amount) ;```
look at the way i pass the pointer and call the function

use semi-colons in the for loop

Code:
```    int sum(int *a,int array_amount)
{
int b, total = 0;
for (b = 0; b <= array_amount; b++ )
{
total+=a[b];
}

}```
Code:
```Enter an integer, negative or positive:
-4
The sum of negative integers is -4.
The sum of positive integers is 0.

0 is a larger number than -4.```
Last edited by wiel; Mar 11, 2012 at 10:20 PM.  Reply With Quote

3. Hardcore Techie Join Date
Jul 2002
Posts
1,391
Rep Power
0

##  Originally Posted by wiel 1. when you assign the arrays to the pointers you dont need the []
2. the function calls dont need data types on them.
3. you are returning values without storing them. plus_total = total ; and minus_total = total; cant see the "total" inside your sum() function.

should be:
Code:
```minus_total =  sum(a,array_amount);

plus_total = sum(a,array_amount) ;```
4. look at the way i pass the pointer and call the function

use semi-colons in the for loop

Code:
```    int sum(int *a,int array_amount)
{
int b, total = 0;
for (b = 0; b <= array_amount; b++ )
{
total+=a[b];
}

}```
Code:
```Enter an integer, negative or positive:
-4
The sum of negative integers is -4.
The sum of positive integers is 0.

0 is a larger number than -4.```
Thanks! Almost like you one of LLVM/Clang developers .  Reply With Quote

4. Hardcore Techie Join Date
Jul 2002
Posts
1,391
Rep Power
0

##  Originally Posted by wiel;733573
[CODE
Enter an integer, negative or positive:
-4
The sum of negative integers is -4.
The sum of positive integers is 0.

0 is a larger number than -4.[/CODE]
If you use a positive integer the result ends up always being 10xx, xx being the true result minus 1 I think. I don't see where the fault is yet ...

Seems to be the 999 to end the loop.

Solved! http://pastebin.com/WBj0qGZH
Last edited by carey; Mar 12, 2012 at 12:31 AM.  Reply With Quote

5. Techie Join Date
May 2003
Posts
229
Rep Power
0

## I noticed a few major problems with this program.

1. First problem, I think you are not properly interpreting part of the question.

/*4. write a program which will accept both positive and negative integers
from the user until 999 is entered to stop the prog.
sum up both sets of num separately and determine the smallest and largest num from both sets, display these results*/

I think that "determine the smallest and largest num from both sets" means you should determine the smallest and largest positive integer, and the smallest and largest negative integer. So if the numbers are 7, 9, 12, -5, -8, -6, you should determine that the smallest positive integer is 7 and the largest positive integer is 12 and the smallest negative integer is -8 and the largest negative integer is -5.

2. Your program will include the 999 sentinel in the calculation of the sum of the positive numbers.
In your request loop, you store the number before you check whether it is 999, so 999 will get stored in the positive loop. You should check for 999 before you store the number (and increment the counter). You are also making an indexing error. Remember that if you have N numbers in an array (stored from the beginning) then it will be stored in indexes 0 to N-1. So you your for loop would be as follows:

for(b=0; b<N; b++)
{
//access numbers here
}

notice I use < and not <=.
Of course if you really want to use <= then you could do this

for(b=0; b<=(N-1); b++)
{
//access numbers here
}

3. You should use two counters (one for the positive integer array and the other for the negative integer array). Currently, this is not causing a problem because you have initialized the arrays to zero, and adding zeros does not change the sum.  Reply With Quote

6. Hardcore Techie Join Date
Jul 2002
Posts
1,391
Rep Power
0

##  Originally Posted by sking I noticed a few major problems with this program.

1. First problem, I think you are not properly interpreting part of the question.

/*4. write a program which will accept both positive and negative integers
from the user until 999 is entered to stop the prog.
sum up both sets of num separately and determine the smallest and largest num from both sets, display these results*/

I think that "determine the smallest and largest num from both sets" means you should determine the smallest and largest positive integer, and the smallest and largest negative integer. So if the numbers are 7, 9, 12, -5, -8, -6, you should determine that the smallest positive integer is 7 and the largest positive integer is 12 and the smallest negative integer is -8 and the largest negative integer is -5.
Makes sense! Thanks for the observation.

2. Your program will include the 999 sentinel in the calculation of the sum of the positive numbers.
In your request loop, you store the number before you check whether it is 999, so 999 will get stored in the positive loop. You should check for 999 before you store the number (and increment the counter). You are also making an indexing error. Remember that if you have N numbers in an array (stored from the beginning) then it will be stored in indexes 0 to N-1. So you your for loop would be as follows:

for(b=0; b<N; b++)
{
//access numbers here
}

notice I use < and not <=.
Of course if you really want to use <= then you could do this

for(b=0; b<=(N-1); b++)
{
//access numbers here
}

3. You should use two counters (one for the positive integer array and the other for the negative integer array). Currently, this is not causing a problem because you have initialized the arrays to zero, and adding zeros does not change the sum.
Trying to work in these new observations into it. Taken up by Computer Architecture now though.  Reply With Quote

7. Hardcore Techie Join Date
Jul 2002
Posts
1,391
Rep Power
0

## The library is about to close, and I have been on my butt all afternoon ...

I altered the code; I want to apply my new-found knowledge of pointers and functions (thanks to fellow forumites :P).

Well, as expected, the code is broken:

http://ideone.com/Bv8wx
http://codepad.org/mQPD6cPF //If Ideone is too slow  Reply With Quote

8. ## What's the error you are getting? not sure what's happening on line 28 and 33 with the pointers(dereference?) but when you add count to plus and minus before they are initialized or are given a value later, that should crash your program neatly.

And the semi-colon on line 67 needs to go
Last edited by wiel; Mar 20, 2012 at 10:04 PM.  Reply With Quote

9. Hardcore Techie Join Date
Jul 2002
Posts
1,391
Rep Power
0

##  Originally Posted by wiel What's the error you are getting? not sure what's happening on line 28 and 33 with the pointers(dereference?) but when you add count to plus and minus before they are initialized or are given a value later, that should crash your program neatly.

And the semi-colon on line 67 needs to go
The program doesn't spout an error. Just hangs after input, then a crash. This happens with both Digital Mars and Gnu C compiler.

If anything, I am going to go over it from scratch. Put all processes in their respective functions. I am just stuck on sort; and research dictates I either go withlearn a hand-coded sort or learn qsort.

I wish sorting was as easy to overcome as a 3 Glasses Puzzle.  Reply With Quote

10. Techie Join Date
May 2003
Posts
229
Rep Power
0

## Ok, there are some major problems with this code.

Firstly, as Wiel has mentioned, you are dereferencing pointers that have not been given a value (which is a big big problem). You didn't respond to what Wiel said so let me be more specific. For example, the following line:

*(plus + count) = &num ;

is telling the computer to
1. Take the address stored in the variable plus and
2. Take the value stored in the variable count and multiply by the size of the type pointed to by the variable plus.
3. Take the result of 2 and add it to the address from 1.
4. Store the result of the right-hand side of the assignment (i.e. &num) in the address resulting from 3.

This is a big problem because your program does not assign an address to plus, so you are storing your values in some unspecified memory location. You need to reserve some memory locations to store the values the user is going to enter. For example, you could do this

int plus_array;
plus = plus_array;

or you can allocate memory from the operating system using the malloc

plus = malloc((sizeof int) * 99);

If you call malloc you may have to include the standard header, using the following line:

#include<stdlib.h>

Ok, first problem solved. The statement actually has another problem. Let me repeat 4 from above, to make is easier to see from here.

4. Store the result of the right-hand side of the assignment (i.e. &num) in the address resulting from 3.

The result of the right hand side of the assignment (i.e. &num) is not the value stored in num (which is the value that the user entered). &num actually results in the address of the variable num.

So you program is not storing the values that the user is entering, you are storing the address of the variable that stores that value (which is NOT the same thing).

Finally, your program still has the problems I pointed out before.  Reply With Quote

#### Posting Permissions

• You may not post new threads
• You may not post replies
• You may not post attachments
• You may not edit your posts
•