Changing array with function when parameter is class type c++

Question

I know it might be a duplicate of this: Changing array inside function in C, but I read it thoroughly and there's one situation that I'm wondering:

void addCustomer(Customer*** array, int length, char type) {
    for (int i = 0; i < length; i++) {

    Customer *c1 = new Customer();

    switch (type) {
    case '1':
        c1 = new LowCustomer();
        break;
    case '2':
        c1 = new MiddleCustsomer();
        break;
    case '3':
        c1 = new HighCustomer();
        break;
    case '4':
        c1 = new VIPCustomer();
        break;
    }

    (*array)[i] = c1;
}

LowCustomer, MiddelCustomer, HighCustomer and VIPCustomer classes are all derived from Customer class.

So, as I had to use the new keyword, the array now had to be ***. Is there a better way? Because my code keeps producing runtime error with 0xcdcdcdcd on c1.

My code in main looks like this:

Customer*** low = new Customer**[10];
addCustomer(low, 10, 'c');

P.S. Oh! BTW I know using vector would make it way easier, but I really want to use pointer for this one.

Please use [std::unique_ptr](https://en.cppreference.com/w/cpp/memory/unique_ptr) or [std::shared_ptr](https://en.cppreference.com/w/cpp/memory/shared_ptr) and try to use std::vector — Gelldur, Dec 18 '18 at 08:57
This is not going to solve your problem but in terms of efficency, instead of doing the `Customer *c1 = new Customer();`and then specifying later the type, why don't you trying using the **Strategy Design Pattern** . There are plenty of examples (ducks are the most understandable thing). — M.K, Dec 18 '18 at 09:05
When you will delete the unused instance of "Customer" which is created before the switch? You simply overwrite the pointer and leak your memory. "***" is very hot, never seen that before! Why you tag your question c++? You only use C here? Intended? Oh, "new" is the only c++ I can see... — Klaus, Dec 18 '18 at 09:05
@Hellowhatsup Can you elaborate on why std::vector would not be acceptable? — JVApen, Dec 18 '18 at 09:19

Max Langhof · Answer 1 · 2018-12-18T08:58:13.343

Customer*** low = new Customer**[10];

This only creates an array of 10 Customer**. Those are uninitialized pointers. In (*array)[i] = c1; you are dereferencing these uninitialized pointers, which is undefined behavior and gets you the runtime errors you see (if you are lucky).

You are also heavily leaking memory here. If you new something, you must delete it eventually. But Customer *c1 = new Customer(); has no corresponding delete, you're just overwriting (or trying to) that pointer later.

If you really wanted to stick with pointers, this would basically work (using a reference to the array of Customer*):

Customer** low = new Customer*[10];
addCustomer(low, 10, 'c');

// ...

void addCustomer(Customer**& array, int length, char type)
{
  for (int i = 0; i < length; i++)
  {
    Customer* c1;
    switch (type) {
    case '1':
        c1 = new LowCustomer();
        break;
    case '2':
        c1 = new MiddleCustsomer();
        break;
    case '3':
        c1 = new HighCustomer();
        break;
    case '4':
        c1 = new VIPCustomer();
        break;
    default:
        // Error handling...
  }

  array[i] = c1;
}

You could also stick with the three-star approach but low must be a Customer**, then you pass a pointer to low to addCustomer: addCustomer(&low, 10, 'c');

Needless to say, use vectors. It's impossible to tell that only the middle * is supposed to be an array by looking at the function signature.

score 1 · Answer 2 · answered Dec 18 '18 at 09:41

I'm explicitly ignoring your comment about std::vector as I haven't seen a good reason you are avoiding this. std::vector Nicely manages the memory of the allocated memory. By storing std::unique_ptr into it, you have the right memory management for the created instances as well.

Some other changes include:

Returning by value instead of an output argument
Reserving the vector, to get a single memory allocation of the vector
Use size_t instead of int for the size to match the size_type of vector

Suggested changes:

Replace 'char' by a scoped enumeration (enum class)

Code on Compiler Explorer

#include <vector>
#include <memory>

struct Customer
{
    virtual ~Customer() = default;
};

struct LowCustomer : Customer {};
struct MiddleCustomer : Customer {};
struct HighCustomer : Customer {};
struct VIPCustomer : Customer {};

using Customers = std::vector<std::unique_ptr<Customer>>;

Customers createCustomer(size_t length, char type)
{
    auto values = Customers{};
    values.reserve(length);
    for (size_t i = 0; i < length; i++)
    {
        switch (type)
        {
        case '1':
            values.emplace_back(std::make_unique<LowCustomer>());
            break;
        case '2':
            values.emplace_back(std::make_unique<MiddleCustomer>());
            break;
        case '3':
            values.emplace_back(std::make_unique<HighCustomer>());
            break;
        case '4':
            values.emplace_back(std::make_unique<LowCustomer>());
            break;
        default:
            values.emplace_back(std::make_unique<Customer>());
            break;// Error handling...
        }
    }

    return values;
}

int main(int, char**)
{
    auto low = createCustomer(10, 'c');
}

Changing array with function when parameter is class type c++

2 Answers2