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;
}
Minas Mina
  • 1,813
  • 2
  • 18
  • 28
Hassan
  • 21
  • 1
  • 1
  • 3

3 Answers3

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.

Minas Mina
  • 1,813
  • 2
  • 18
  • 28
  • Although answering the question, this is really C-style coding. In modern C++, the use of `new` and `delete` are even [considered 'bad'](https://www.quora.com/Why-are-the-%E2%80%98new%E2%80%99-and-%E2%80%98delete%E2%80%99-keywords-considered-bad-in-modern-C++) – JHBonarius Feb 28 '18 at 10:49
  • Thanks a lot. But I'm required to use a void function so I can't return anything from that function. – Hassan Feb 28 '18 at 14:24
  • @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. – Minas Mina Feb 28 '18 at 14:26
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.

Jorge Bellon
  • 2,369
  • 10
  • 22
  • That was really simultaneously. :-) – schorsch312 Feb 28 '18 at 09:47
  • 1
    Oh boy, I almost fell for that one too. Can you spot the bug? ;) – Quentin Feb 28 '18 at 09:49
  • Yes. It is not `std::fill`. My memory has betrayed me :( – Jorge Bellon Feb 28 '18 at 09:53
  • Hah, I actually autocorrected that in my head while reading it, didn't notice. Not, it's more subtle! – Quentin Feb 28 '18 at 09:56
  • 1
    please dont turn SO into a code writing service. At least you should explain what is wrong with OPs code – 463035818_is_not_a_number Feb 28 '18 at 09:56
  • 2
    @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 :) – Quentin Feb 28 '18 at 10:12
  • Although the compiler will do it for you, I would not create the variables `first` and `last`. Just write `std::copy(numbers.begin(), numbers.end(), std::back_inserter(numbers) );`. It is very readable and maintainable. – JHBonarius Feb 28 '18 at 10:33
  • Thanks a lot. It is required from me that I use this approach and not vectors, even though the latter is much easier and more suitable for this task. I'll try passing a pointer to a pointer and hopefully it works. – Hassan Feb 28 '18 at 14:23
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;
}
schorsch312
  • 5,032
  • 4
  • 21
  • 47
  • please dont turn SO into a code writing service. At least you should explain what is wrong with OPs code – 463035818_is_not_a_number Feb 28 '18 at 09:56
  • 1
    @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. – JHBonarius Feb 28 '18 at 10:35
  • 1
    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 – 463035818_is_not_a_number Feb 28 '18 at 10:37
  • ..btw I never said that an example using a vector is useless, it just doesnt answer the question – 463035818_is_not_a_number Feb 28 '18 at 10:37
  • 1
    @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 ;) – JHBonarius Feb 28 '18 at 10:39
  • minas answer starts with "The problem is .." explaining what is wrong with OPs code, that is what is imho missing in yours. Nevermind, I cannot convince you but I can give your answer a downvote, case closed ;) – 463035818_is_not_a_number Feb 28 '18 at 10:52