Dynamically allocate memory, error in copy constructor

Question

I have homework to use dynamically allocated memory. My Professor gave me some instructions. Using them, I coded the below code. I am getting an error at random times. Some times the error appears before copy execution. Some times it will copy one object and wont copy the next one. I don`t understand what I am doing wrong.

Default constructor

GroceryItem::GroceryItem()
{
    item_name = new char[strlen("") + 1];
    strcpy(item_name, "");
    item_price = 0;
    qty_on_hand = 0;
    qty_purchased = 0;
};

Function below is copy constructor that I use to copy two objects:

GroceryItem::GroceryItem(const GroceryItem& Grocery_in)
{
    item_name = new char[strlen(Grocery_in.item_name) + 1];
    strcpy(item_name, Grocery_in.item_name);
    item_price = Grocery_in.item_price;
    qty_on_hand = Grocery_in.qty_on_hand;
    qty_purchased = Grocery_in.qty_purchased;
}
;

below is assigment opperator

GroceryItem& GroceryItem::operator=(GroceryItem& copy_item)
{
    if (this == &copy_item)
        return *this;
    else
    {
        delete[] item_name;
        item_name = new char[strlen(copy_item.item_name)+1];
        strcpy(item_name, copy_item.item_name);
        item_price = copy_item.item_price;
        qty_on_hand = copy_item.qty_on_hand;
        qty_purchased = copy_item.qty_purchased;
        return *this ;      // They are the same
    }
}

Calls out from function below when I try to copy to temp2:

void sort_items(GroceryItem ini_customer_GroceryItem[], int number)
{
    int j = 0, k = 0;
    GroceryItem temp2;

    for (j = 0; j < number - 1; j++)    // n-1 passes
    {
        for (k = number - 1; j < k; k--)    // each pass runs one fewer than the preceding one
        {
            if (ini_customer_GroceryItem[k - 1] > ini_customer_GroceryItem[k])
            {
                temp2 = ini_customer_GroceryItem[k - 1];
                ini_customer_GroceryItem[k - 1] = ini_customer_GroceryItem[k];
                ini_customer_GroceryItem[k] = temp2;
            }
        }
    }
}

Here is the error

Answer 1

Your sort_items() function should use the std::swap() algorithm instead of copying objects manually:

/*
temp2 = ini_customer_GroceryItem[k - 1];
ini_customer_GroceryItem[k - 1] = ini_customer_GroceryItem[k];
ini_customer_GroceryItem[k] = temp2;
*/
std::swap(ini_customer_GroceryItem[k - 1], ini_customer_GroceryItem[k]);

Either way, you did not implement a copy constructor, only a copy assignment operator (and copy_item should be const in your implementation). See Rule of Three. You need to implement a proper copy constructor:

GroceryItem::GroceryItem(const GroceryItem& source_item)
{
    item_name = new char[strlen(source_item.item_name)+1];
    strcpy(item_name, source_item.item_name);
    item_price = source_item.item_price;
    qty_on_hand = source_item.qty_on_hand;
    qty_purchased = source_item.qty_purchased;
}

And then you can implement your copy assignment operator using the copy constructor:

GroceryItem& GroceryItem::operator=(const GroceryItem& copy_item)
{
    if (this != &copy_item)
    {
        GroceryItem temp(copy_item);
        std::swap(temp, *this);
    }
    return *this;
}

Which can be simplified to:

GroceryItem& GroceryItem::operator=(GroceryItem copy_item)
{
    std::swap(copy_item, *this);
    return *this;
}

And, of course, don't forget a destructor, if you have not already implemented one:

GroceryItem::~GroceryItem()
{
    delete[] item_name;
}

And an operator>(), of course, since sort_items() expects one.

Now, with that all said, if you change the item_name member to be a std::string instead of a char*, you won't need to manually implement a destructor, copy constructor, or copy assignment operator at all (just the default constructor to zero-initialize the numeric members). The compiler's default generated implementations for the destructor, copy constructor, and copy assignment operator will suffice for managing all of the data members for you:

class GroceryItem
{
public:
    std::string item_name;
    float item_price;
    int qty_on_hand;
    int qty_purchased;

    GroceryItem();

    bool operator > (const GroceryItem& item) const;
};

GroceryItem::GroceryItem()
{
    item_price = 0.0f;
    qty_on_hand = 0;
    qty_purchased = 0;
};

bool GroceryItem::operator > (const GroceryItem& item) const
{
    return ...;
}

void sort_items(GroceryItem ini_customer_GroceryItem[], int number)
{
    int j = 0, k = 0;
    //GroceryItem temp2;

    for (j = 0; j < number - 1; j++)    // n-1 passes
    {
        for (k = number - 1; j < k; k--)    // each pass runs one fewer than the preceding one
        {
            if (ini_customer_GroceryItem[k - 1] > ini_customer_GroceryItem[k])
            {
                /*
                temp2 = ini_customer_GroceryItem[k - 1];
                ini_customer_GroceryItem[k - 1] = ini_customer_GroceryItem[k];
                ini_customer_GroceryItem[k] = temp2;
                */
                std::swap(ini_customer_GroceryItem[k - 1], ini_customer_GroceryItem[k]);
            }
        }
    }
}