0

I'm trying to write a C++ program that adds elements in an array which are not divisible by 2. i initialized my array in the main function and created another function in which using a for loop and an if statement, I try to extract the numbers in the array which are not divisible by 2. Everything seems to fine until i run the program and it prints several numbers which i can't seem to make any sense out of

#include <iostream>
#include <string.h>

using namespace std;
//sum of all numbers that are not divisible by 2
int findDivisibleNumbers(int a[], int n, int sumofnumbers)
{
    for (int i = 0; i < n; i++) {
        if (i % 2 != 0)
            sumofnumbers = sumofnumbers + a[i];

            cout << "Sum of values not divisible by 2 = " << sumofnumbers << endl;
    }
    return 0;
}

int main()
{
`  int arrayOfCrazyNumbers[] = { 11,2446,2343,144,65,26,17,8,29,10,238126,1912338 };
   findDivisibleNumbers(arrayOfCrazyNumbers, sizeof(arrayOfCrazyNumbers), 0);

}

when i run the program its prints this output;

Sum of values not divisible by 2 = 0
Sum of values not divisible by 2 = 2446
Sum of values not divisible by 2 = 2446
Sum of values not divisible by 2 = 2590
Sum of values not divisible by 2 = 2590
Sum of values not divisible by 2 = 2616
Sum of values not divisible by 2 = 2616
Sum of values not divisible by 2 = 2624
Sum of values not divisible by 2 = 2624
Sum of values not divisible by 2 = 2634
Sum of values not divisible by 2 = 2634
Sum of values not divisible by 2 = 1914972
Sum of values not divisible by 2 = 1914972
Sum of values not divisible by 2 = -857078488
Sum of values not divisible by 2 = -857078488
Sum of values not divisible by 2 = -1716071948
Sum of values not divisible by 2 = -1716071948
Sum of values not divisible by 2 = -1716071937
Sum of values not divisible by 2 = -1716071937
Sum of values not divisible by 2 = 1738090781
Sum of values not divisible by 2 = 1738090781
Sum of values not divisible by 2 = 1739319245
Sum of values not divisible by 2 = 1739319245
Sum of values not divisible by 2 = 1743381485
Sum of values not divisible by 2 = 1743381485
Sum of values not divisible by 2 = 1743381486
Sum of values not divisible by 2 = 1743381486
Sum of values not divisible by 2 = 1754875318
Sum of values not divisible by 2 = 1754875318
Sum of values not divisible by 2 = 1766408486
Sum of values not divisible by 2 = 1766408486
Sum of values not divisible by 2 = 1770470818
Sum of values not divisible by 2 = 1770470818
Sum of values not divisible by 2 = 929666308
Sum of values not divisible by 2 = 929666308
Sum of values not divisible by 2 = 930850097
Sum of values not divisible by 2 = 930850097
Sum of values not divisible by 2 = 930850097
Sum of values not divisible by 2 = 930850097
Sum of values not divisible by 2 = 930850097
Sum of values not divisible by 2 = 930850097
Sum of values not divisible by 2 = 930850097
Sum of values not divisible by 2 = 930850097
Sum of values not divisible by 2 = 930850097
Sum of values not divisible by 2 = 930850097
Sum of values not divisible by 2 = 932097113
Sum of values not divisible by 2 = 932097113
Sum of values not divisible by 2 = 932097113

C:\COM205_ProgrammingTwo2021Q1\CPlusPlus\BasicDataTypeMemoryAndPointersPt1\Debug\BasicDataTypeMemoryAndPointersPt1.exe (process 21016) exited with code 0.
To automatically close the console when debugging stops, enable Tools->Options->Debugging->Automatically close the console when debugging stops.
Press any key to close this window . . .

what could be the problem?

Khadijah Kamba
  • 21
  • 2
  • 4
    `sizeof(arrayOfCrazyNumbers)` is the length of the array in bytes, not the number of elements. By the way, in C++ better use `std:array` than raw C arrays. – Werner Henze Feb 27 '21 at 15:21
  • https://stackoverflow.com/questions/33523585/how-do-sizeofarr-sizeofarr0-work#:~:text=sizeof(arr)%20is%20the%20total,if%20the%20array%20itself%20exists). – fulaphex Feb 27 '21 at 15:21
  • `sizeof arrayOfCrazyNumbers / sizeof *arrayOfCrazyNumbers` will provide the number of elements. – David C. Rankin Feb 27 '21 at 20:41
  • Why are you passing a counter to the function from main if the counter is not going to be used in main. I would either define it in your main and return the value from the function, or pass the counter by reference and print it in the function. – The_Redhawk Feb 27 '21 at 21:23

4 Answers4

1

sizeof returns you the size in bytes (more here). To get the number of elements you can use sizeof(arrayOfCrazyNumbers)/sizeof(arrayOfCrazyNumbers[0]), this divides the total number of bytes by the number of bytes of a single element of the array.

#include <iostream>
#include <string.h>

using namespace std;
//sum of all numbers that are not divisible by 2
int findDivisibleNumbers(int a[], int n, int sumofnumbers)
{
    for (int i = 0; i < n; i++) {
        if (i % 2 != 0)
            sumofnumbers = sumofnumbers + a[i];

    }
    cout << "Sum of values not divisible by 2 = " << sumofnumbers << endl;
    return 0;
}

int main()
{
    int arrayOfCrazyNumbers[] = { 11,2446,2343,144,65,26,17,8,29,10,238126,1912338 };
    findDivisibleNumbers(arrayOfCrazyNumbers, sizeof(arrayOfCrazyNumbers)/sizeof(arrayOfCrazyNumbers[0]), 0);
}

You can also change you code to use https://en.cppreference.com/w/cpp/container/array or https://en.cppreference.com/w/cpp/container/vector

Using vectors, the code would look like this:

#include <iostream>
#include <string.h>
#include <vector>

using namespace std;
//sum of all numbers that are not divisible by 2
int findDivisibleNumbers(vector<int> a, int n, int sumofnumbers)
{
    for (int i = 0; i < n; i++) {
        if (i % 2 != 0)
            sumofnumbers = sumofnumbers + a[i];

    }
    cout << "Sum of values not divisible by 2 = " << sumofnumbers << endl;
    return 0;
}

int main()
{
   vector<int> arrayOfCrazyNumbers = { 11,2446,2343,144,65,26,17,8,29,10,238126,1912338 };
   findDivisibleNumbers(arrayOfCrazyNumbers, arrayOfCrazyNumbers.size(), 0);
}
fulaphex
  • 2,879
  • 3
  • 19
  • 26
  • Well, using vectors, you wouldn't need to pass a "number of elements" parameter. Your version still has the feature where it prints once per odd number instead of once at the end, and the `sumofnumbers` parameter is still not very useful. – Nathan Pierson Feb 27 '21 at 16:21
1

As stated all over the place, the biggest issue is your misuse of sizeof(). sizeof() tells you the size of something in bytes. If you wanted the length of your array and wanted to continue to use sizeof(), you need to consider the size of each element.

sizeof(arrayOfCrazyNumbers) / sizeof(arrayOfCrazyNumbers[0])

That's the total bytes of the array divided by the bytes in an int (likely 4), giving you the number ints in your array. That gets you the correct range, but you're summing the wrong elements.

The issue is i % 2 != 0. i is just a counting number, not an element of your array. You want a[i] % 2. You can leave out the != 0, it's redundant.

Those two changes to your code should fix your problems.

BUT WAIT, you tagged this C++, and that's a very C way of doing things. You could use std::array, which knows its own size, and doesn't require you to calculate it or pass it along.

You also need to make better use of your function. The name is off. Its job really isn't to find divisible numbers, its job is to sum odd numbers. So we'll change the name. I'm choosing sum_odds. You pass a variable to contain the sum as an argument, but you pass it by value instead of by reference, and you also marked the function as returning an int, but just return 0 no matter what. Either pass the sum variable by reference (output parameter), or actually return the sum and not 0. main() should always return 0, and it's an exception to how we should write functions. I will choose to return the sum and do my printing in main. After all, the new function name isn't sum_odds_and_print.

Also don't using namespace std;. It's bad practice, and a hard habit to kick.

#include <array>
#include <iostream>

// sum of all numbers that are not divisible by 2
int sum_odds(const std::array<int, 12>& a) {
  int oddSum = 0;
  for (auto i : a) {  // range-based for loop
    if (i % 2) oddSum += i;
  }
  return oddSum;
}

int main() {
  std::array<int, 12> arrayOfCrazyNumbers{11, 2446, 2343, 144, 65,     26,
                                          17, 8,    29,   10,  238126, 1912338};
  int oddSum = sum_odds(arrayOfCrazyNumbers);
  std::cout << "Sum of odd numbers: " << oddSum << '\n';
}

There is one other change to the code, and that's the use of a range-based for loop. It's a shorthand when you want to go over every element. In this case, i is not a counting variable anymore, but i will hold the value of each element of the array as it loops.

I still wouldn't call this ideal, since the array size is part of its definition. I could use a constant or a macro, but meh. We'll switch from std::array to std::vector. For all intents and purposes of this program, we can imagine it the same as an array, except I don't have to specify the size all the time, and it knows its own size.

#include <iostream>
#include <vector>

// sum of all numbers that are not divisible by 2
int sum_odds(const std::vector<int>& a) {
  int oddSum = 0;
  for (auto i : a) {  // range-based for loop
    if (i % 2) oddSum += i;
  }
  return oddSum;
}

int main() {
  std::vector<int> arrayOfCrazyNumbers{11, 2446, 2343, 144, 65,     26,
                                          17, 8,    29,   10,  238126, 1912338};
  int oddSum = sum_odds(arrayOfCrazyNumbers);
  std::cout << "Sum of odd numbers: " << oddSum << '\n';
}

That's a pretty good spot to call it, but just for kicks, let's say we decided to take further advantage of the Standard Library. There's a a few different ways to go about this, here's one:

#include <iostream>
#include <numeric>  // std::accumulate()
#include <vector>

// sum of all numbers that are not divisible by 2
int sum_odds(const std::vector<int>& a) {
  return std::accumulate(a.begin(), a.end(), 0,
                         [](int sum, int i) { return sum += i % 2 ? i : 0; });
}

int main() {
  std::vector<int> arrayOfCrazyNumbers{11, 2446, 2343, 144, 65,     26,
                                       17, 8,    29,   10,  238126, 1912338};
  int oddSum = sum_odds(arrayOfCrazyNumbers);
  std::cout << "Sum of odd numbers: " << oddSum << '\n';
}

I know that this probably looks like gibberish, and that's fine. As you continue to study programming and [hopefully] C++, this will make more sense. The craziest syntax can be learned about by looking up lambdas and the ternary operator. I recommend the site https://cppreference.com .

sweenish
  • 4,793
  • 3
  • 12
  • 23
1

Another quick way to approach the problem is using std::array combined with std::for_each (which you can use with your plain-old-array as well). Using this approach you simply pass a reference to the array to your function and use std::for_each() to iterate over each element, summing the odd elements. Returning the sum makes the function a bit more useful.

For example you could do:

#include <iostream>
#include <iomanip>
#include <array>
#include <algorithm>

//sum of all numbers that are not divisible by 2
int findDivisibleNumbers (std::array<int,12>& a)
{
    int sum = 0;
    std::for_each (a.begin(), a.end(), [&sum](int &n) {
                                            if (n & 1) {    /* is n odd? */
                                                sum += n;   /* add to sum */
                                                std::cout << "sum += " << std::setw(7) <<
                                                            n << " = " << sum << '\n';
                                            }
                                        });
    
    return sum;     /* return sum */
}

int main (void) {
    
    std::array<int,12> arrayOfCrazyNumbers =  { 11, 2446, 2343, 144, 65, 26, 
                                                17, 8, 29, 10, 238126, 1912338 };
    int sum = findDivisibleNumbers (arrayOfCrazyNumbers);
    
    std::cout << "\ntotal sum: " << sum << '\n';
}

Example Use/Output

$ ./bin/sum_not_divisible_by_2
sum +=      11 = 11
sum +=    2343 = 2354
sum +=      65 = 2419
sum +=      17 = 2436
sum +=      29 = 2465

total sum: 2465

(note: it is a bit odd to name a function findDivisibleNumbers() that is supposed to find and sum numbers that are NOT divisible by 2.... just saying...)

Let me know if you have further questions.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
0

Your print call is in the for loop. The n variable you specified in your function is right it's just the value you put in the function that isn't right. sizeof() will give the size off the array in bytes instead of the number of values that is in the array, easy way to fix this is to put in (sizeof(array) / sizeof(int)). This will divide the size off bytes of the array by the size off one int. In the if statement we want to check if the number from the array at the i index is divisible by 2 or not instead of i itself. The sumofnumber variable we can create inside the function itself.

#include <iostream>
#include <string.h>

using namespace std;
//sum of all numbers that are not divisible by 2
int findDivisibleNumbers(int a[], int n)
{
    int sumofnumbers = 0;
    for (int i = 0; i < n; i++) {
        if (a[i] % 2 != 0)
            sumofnumbers = sumofnumbers + a[i];
    }
    cout << "Sum of values not divisible by 2 = " << sumofnumbers << endl;
    return 0;
}

int main()
{
    int arrayOfCrazyNumbers[] = { 11,2446,2343,144,65,26,17,8,29,10,238126,1912338 };
    findDivisibleNumbers(arrayOfCrazyNumbers, sizeof(arrayOfCrazyNumbers) / sizeof(int));

}

My output:

Sum of values not divisible by 2 = 2465
Gideon
  • 36
  • 1