Page 1 of 4 123 ... LastLast
Results 1 to 10 of 40

Thread: What is wrong with this code?

  1. #1
    Join Date
    Jul 2002
    Posts
    1,388
    Rep Power
    0

    Default What is wrong with this code?

    Please give an open critique of this code?

    http://pastebin.com/yhQ0QaXy
    Sharing is caring? Then what if I care too much? A precarious situation, indeed...
    http://ip-check.org/ip-check.png

  2. #2
    Join Date
    Mar 2006
    Posts
    325
    Rep Power
    0

    Default

    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];
            } 
                return total;
    
        }
    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 11:20 PM.
    1337

  3. #3
    Join Date
    Jul 2002
    Posts
    1,388
    Rep Power
    0

    Default

    Quote Originally Posted by wiel View Post
    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];
              } 
                  return total;
      
          }
      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 .
    Sharing is caring? Then what if I care too much? A precarious situation, indeed...
    http://ip-check.org/ip-check.png

  4. #4
    Join Date
    Jul 2002
    Posts
    1,388
    Rep Power
    0

    Default

    Quote 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 ...

    Addendum

    Seems to be the 999 to end the loop.

    Solved! http://pastebin.com/WBj0qGZH
    Last edited by carey; Mar 12, 2012 at 01:31 AM.
    Sharing is caring? Then what if I care too much? A precarious situation, indeed...
    http://ip-check.org/ip-check.png

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

    Default

    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.

  6. #6
    Join Date
    Jul 2002
    Posts
    1,388
    Rep Power
    0

    Default

    Quote Originally Posted by sking View Post
    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.
    Sharing is caring? Then what if I care too much? A precarious situation, indeed...
    http://ip-check.org/ip-check.png

  7. #7
    Join Date
    Jul 2002
    Posts
    1,388
    Rep Power
    0

    Default

    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

    Please analyse this?
    Sharing is caring? Then what if I care too much? A precarious situation, indeed...
    http://ip-check.org/ip-check.png

  8. #8
    Join Date
    Mar 2006
    Posts
    325
    Rep Power
    0

    Default

    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 11:04 PM.
    1337

  9. #9
    Join Date
    Jul 2002
    Posts
    1,388
    Rep Power
    0

    Default

    Quote Originally Posted by wiel View Post
    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.
    Sharing is caring? Then what if I care too much? A precarious situation, indeed...
    http://ip-check.org/ip-check.png

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

    Default

    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[99];
    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.

Posting Permissions

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