Adding to array of pointers causes segmentation fault

[eagerissac]

I'm creating a program that resembles a store which manages products and customers. I'm having trouble with adding customer objects to the store as it keeps causing a segmentation fault. The store should include a collection of its customers stored as a dynamically allocated array of pointers ordered by id when they're printed out.

It appears the line customers[index] = customer from add() in CustomerArray.cc is causing a segmentation fault as when I comment it out, the error goes away. I thought it might've been because my pointer wasn't pointing to anything originally, so I set each element in customers to NULL in the default constructor.

Despite making these changes, the segmentation fault still occurs. I was wondering if anyone knows why? I'm really frustrated as I have several other functions to add but can't begin adding them until I get this part to work.

CustomerArray.cc

	void CustomerArray::add(Customer* customer){
		int index = 0;
		if(numCustomers < MAX_CUSTOMERS){
			for(int i = 0; i <= numCustomers-1; ++i){ 
				if(customers[i]->getNextId() < customer->getNextId()){  
					for(int j = numCustomers-1; j >= i; --j){
						customers[j+1] = customers[j]; 
					}
					index = i;
					break;
				}
			}
			customers[index] = customer; //something wrong with this line
			++numCustomers;
		}
	}
		

[TheIdeasMan]

numCustomers is 0 , so most of your loops don't do anything, maybe you meant MAX_CUSTOMERS instead.

But my best advice is to not use pointers or new at all, consider using std::vector<Customer> instead. Pointer to pointer code is notoriously error prone.

If you are worried about dynamic memory, std::vector does it for you. You really will save yourself a lot of hassle, code simpler, less problems, easier all round.

Instead of NULL use nullptr

avoid #define for constants, use constexpr instead:

constexpr int MAX_CUSTOMERS {10};

Try to avoid using namespace std; too, type std:: before every std thing. Sounds difficult, but it really is the best way IMHO. That's what professional coders do.

Good Luck !!

[dutch]

Do you perhaps mean something like getId() instead of getNextId() ?

void CustomerArray::add(Customer* customer) {
    if (numCustomers >= MAX_CUSTOMERS) {
        return; // you may want to do something to indicate the overflow (report error; exit; return a bool;...)
    }
    int i = 0;
    for ( ; i < numCustomers && customers[i]->getId() < customer->getId(); ++i)
        ;
    for (int j = numCustomers; j > i; --j)
        customers[j] = customers[j - 1]; 
    customers[i] = customer;
    ++numCustomers;
}