CODEWITHSUNDEEP
c++
mansdz
mansdz

Reputation: 1

Dynamically allocate array in functions

So, I am new to c++ and I would like to know why the value doesn't get updated in the array. Can anybody please explain where I Did wrong. Would appreciate any commends. when I try to execute it, it always returns 0 as in std::cout<<sum<<std::endl; I try to see and I realize that my array doesn't get updated

    #include <iostream>
    int sum_of_even(int* arr, int n)
    {
        int sum = 0;
        arr = new int[n];
        for(int i=0;i<n;i++)
        {
            if(arr[i] % 2==0)
            {
                sum += arr[i];
                std::cout<<sum<<std::endl;
            }
        }
        return sum;
    }
    int main()
    {
        int numbers[5] = {1,2,3,4,5};
        std::cout<<sum_of_even(numbers,5)<<std::endl;
    }

Upvotes: 0

Views: 1741

Answers (5)

Muhammad Mujtaba Shafique
Muhammad Mujtaba Shafique

Reputation: 295

You are declaring the array again in your function due to which the array you passed is not even being used in your function. An uninitialized array is being used in your function making your sum carry wrong value

Upvotes: 0

Ruks
Ruks

Reputation: 3956

When arrays condense into pointers in a function parameter, it stores the addresses to the first element in the array. Addresses are just locations in memory, and nothing more, they have nothing to do with their contents, so what this line does:

arr = new int[n];

This just makes the whole passing the pointer to the first element of the array useless since the first thing you do with the pointer is make it point to a different memory that was allocated using new[] that is completely unrelated to the array you pass to the function.

Additionally, new[] will not magically initialize all your elements, so what you see are uninitialized/undefined values inside the dynamically allocated memory, and trying to read uninitialized values is Undefined Behavior in C++ because you can get any sort of value out of it.

So you have to initialize the memory manually after allocating it:

#include <iostream>
#include <algorithm>

int sum_of_even(int* arr, int n)
{
    int sum = 0;
    int* const tmp = arr; // Store the pointer to the first element of the array temporarily as the line below will immediately replace it
    arr = new int[n];
    std::copy(tmp, tmp + n, arr); // Initialize the dynamically allocated memory with values inside 'arr'
    for(int i = 0; i < n; i++)
    {
        if(arr[i] % 2 == 0)
        {
            sum += arr[i];
            std::cout << sum << std::endl;
        }
    }
    delete[] arr; // You NEED to call `delete[]` after using `new[]` or say hello to memory leaks!
    return sum;
}

// ...

Or just get rid of the dynamic allocation entirely since it is not needed and adds needless overhead.

Edit: It appears that you are trying to allocate at the address where the array is situated. You can do that using placement new:

arr = new (arr) new int[n];

But this is kind of useless and isn't really needed in your case.

Upvotes: 1

Jan Gabriel
Jan Gabriel

Reputation: 1186

You are re-assigning the int* arr pointer to point to a different memory address created on the heap with new int[n] in this line:

arr = new int[n];

You can see your code working as expected here when commenting out that line, which means you are not accessing the actual numbers array.

It is also good practice to indicate the intention of your function through using const when passing parameters. E.g. if you want to safely prevent the re-assigning of the pointer inside your function, thus indicating "read-only", use this example:

// The order of const and the type int* matters here
int sum_of_even(int* const arr, int n)

/* Compiler error:
<source>: In function 'int sum_of_even(int*, int)':
<source>:5:13: error: assignment of read-only parameter 'arr'
         arr = new int[n]; */

As an added note. Your function creates a memory leak, due to not freeing the new int[n] array. This is done through adding delete [] arr;

I would also propose to solve your problem in a modern cpp way through using the containers and std functions available in the stl.

For example, rather add your numbers to a std::vector container and use std::accumulate in combination with a lambda to sum your even numbers as in this example:

// Use a vector container for your data
std::vector<int> numbers{1, 2, 3, 4, 5};

// Create the sum with a combination of accumulate and a sum if even lambda
int sum = std::accumulate(numbers.begin(), 
                          numbers.end(),
                          0,  // Start with a sum of zero
                          [](int sum, int num) {
                           // Sum only on EVEN numbers
                           return num % 2 == 0 ? sum + num : sum;
                         });

Upvotes: 0

T. Fairchild
T. Fairchild

Reputation: 1

You are passing the address of the array, not the array itself. You can see this if you add a debug statement printing the value of arr immediately inside sum_of_even. You will see something like 0x7fff1c32e890 displayed. This is the machine address where arr is stored.

Change sum_of_even to receive an array, not a pointer:

int sum_of_even(int arr[], int n)

and as previously mentioned, this line wipes out your entire array

arr = new int[n]

Upvotes: 0

the busybee
the busybee

Reputation: 12600

When calling sum_of_even() you provide the address of the first element of numbers as the parameter arr.

But then in sum_of_even() you replace this address with the result of new int[n].

So you don't sum up the values of numbers.

Upvotes: 0

Related Questions