1

I need to transform an array from {4, 2 ,5} to {4, 2, 5, 4, 2, 5}. Here's my output: {4, 2, 5, 3.21143e-322, 0, 2}, which is obviously incorrect. But I cannot seem to figure out the issue in my logic. Perhaps another perspective can find that issue.

This is my code:

void repeatArray(double* oldArray, int size) {
    int newSize = size * 2;
    double* newArray = new double[newSize];
    for (int i = 0; i < size; i++) {
        newArray[i] = oldArray[i];
    }
    for (int i = 0; i < size; i++) {
        newArray[size+i] = oldArray[i];
    }
    oldArray = newArray;
    delete [] newArray;
}

int main() {
    double* oldArray = new double[3];
    oldArray[0] = 4;
    oldArray[1] = 2;
    oldArray[2] = 5;
    repeatArray(oldArray, 3);
    for (int i=0; i<6; i++)
        cout << oldArray[i] << endl;
    delete []oldArray;
    return 0;
}
Improve this question
3
  • 4
    In C++, use the STL containers, i.e. vector<double>. What you are writing is more C without C++... Commented Feb 28, 2018 at 9:35
  • 4
    Use the standard library, they're there for a reason Commented Feb 28, 2018 at 9:38
  • You can either return the pointer to the newly created array, or pass a reference to the oldArray and use a double pointer to access it in your function. Commented Feb 28, 2018 at 9:42

3 Answers 3

Reset to default
6

The problem is that merely assigning the new array to the pointer in repeatArray() does not change it from the outside.

What you can do in repeatArray() is to return the newly created array.

double* repeatArray(double* oldArray, int size) {
    int newSize = size * 2;
    double* newArray = new double[newSize];
    for (int i = 0; i < size; i++) {
        newArray[i] = oldArray[i];
    }
    for (int i = 0; i < size; i++) {
        newArray[size+i] = oldArray[i];
    }
    delete [] oldArray;
    return newArray;
}

And in main():

oldArray = repeatArray(oldArray, 3);

A probably better approach would be to use std::vector that is resized automatically as needed.

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

Although answering the question, this is really C-style coding. In modern C++, the use of new and delete are even considered 'bad'
Thanks a lot. But I'm required to use a void function so I can't return anything from that function.
@Hassan In that case, you can pass a reference pointer to repeatArray: void repeatArray(double*& oldArray, int size). With this, changes to the pointer will be visible outside the method.
1

The problem is that repeatArray parameters are local to the function, so if you change their value, the change won't be seen past the function call. You can use a pointer to pointer double** oldArray to change what oldArray is pointing to, or return the pointer of the new array location.

However, this is C++, where you can use STL containers. Your code would become much more simple and readable.

void repeat( std::vector<double>& numbers ) {
   // Reserve rellocates the vector if there is not enough space
   // otherwise, the iterators first and last could be invalidated.
   numbers.reserve( numbers.size() * 2 );

   auto first = numbers.begin();
   auto last = numbers.end();
   std::copy( first, last, std::back_inserter(numbers) );
}

int main() {
   std::vector<double> nums({4, 2, 5});
   repeat(nums);
   for( double num : nums ) {
      std::cout << num << '\n';
   }
   return 0;
}

std::vector takes care of your allocation, reallocation and copy of the elements, and the deallocation of the resources it makes use.

Improve this answer

8 Comments

That was really simultaneously. :-)
Oh boy, I almost fell for that one too. Can you spot the bug? ;)
Yes. It is not std::fill. My memory has betrayed me :(
please dont turn SO into a code writing service. At least you should explain what is wrong with OPs code
@JorgeBellón ok, I'll drop it: the back_inserter might cause the vector to reallocate, invalidating both first and last and causing UB on the next iteration. You need to reserve first to make sure that doesn't happen :)
|
0

Since you said C++, I suggest you use a proper container, e.g. std::vector.

If you don't have a very good reason to do your own memory management, than you should avoid using new and delete, see here for an elaborated explanation.

Using templates enhances the possibility to reuse the repeat function with other types than double.

Thus, your code is reduced to the following and is easier to read. 

#include <vector>
#include <iostream>
template <typename T>
std::vector<T> repeat(const std::vector<T>& originalVec) {
    auto repeatedVec = originalVec;
    repeatedVec.insert(repeatedVec.end(), originalVec.begin(), originalVec.end());

    return repeatedVec;
}

int main() {
    std::vector<double> oldArray {4,2,5};

    const auto newArray = repeat(oldArray);

    for (const auto item : newArray) {
        std::cout << item << std::endl;
    }

    return 0;
}
Improve this answer

6 Comments

please dont turn SO into a code writing service. At least you should explain what is wrong with OPs code
@user463035818 I disagree with your statement. The OP clearly does not yet know how to use STL. A good example could help there. BTW. You could post your comment to almost every answer on SO... So many times people give code examples.
it does not help OP to understand what is wrong with their code. Next time OP encounters similar problem they will post a similar question again and you will again post working code? this isnt the SO I know
..btw I never said that an example using a vector is useless, it just doesnt answer the question
@user463035818 you could as well state that Minas Mina's answer is 'bad', as he also give the solution in code. Who says the OP will not be back with a similar question anyhow ;)
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.