0

My code:

class item{
int plu;
char * name;
double price;
double inv;
public:
void setPLU(int g) { plu = g; }
void setName(const char * p) { name = copyStr(p); }
void setPrice(double g) { price = g; }
void setInventory(double g) { inv = g; }
int getPlu() { return plu; }
char*getName() { return name; }
double getPrice() { return price; }
double getInventory() { return inv; }
item(){
    name = nullptr;
}
~item(){
    delete name;
}
};
class puItem : public item{
bool type;
public:
void setType(bool g) { type = g; }
bool getType() { return type; }
};
class nodeU{
puItem fruit;
nodeU * next;
public:
nodeU * getNext(){ return next; }
puItem getFruit(){ return fruit; }
void setNext(nodeU * g){ next = g; }
void setFruit(puItem g) { fruit = g; }
nodeU(){
    next = nullptr;
}
};
class linkedListU{
nodeU * head;
int size;
public:
nodeU * getHead(){
    return head;
}

void setHead(nodeU * n){
    head = n;
}
//Append
void appendNode(nodeU * n){
    if (head == nullptr){
        head = n;
    }
    else{
        nodeU * iter = head;
        while (iter){
            iter = iter->getNext();
        }
        iter->setNext(n);
    }
    size++;
}
linkedListU()
{
    head = nullptr;
    size = 0;
}
puItem * pluLookup(int g){
    nodeU * iter = head;
    while (iter)
    {
        if ((iter->getFruit()).getPlu() == g)
            return &(iter->getFruit());
        iter = iter->getNext();
    }
    return nullptr;
}

};
void checkout(linkedListP, linkedListU);
linkedListU unitList;
linkedListP poundList;
nodeU * inputU=new nodeU;
int main()
{
    ifstream infile;
    ofstream outfile;
    int tempPlu;
    string tempName;
    bool tempType;
    double tempPrice, tempInv;
    infile.open("products.txt");
        puItem unit;

        infile >> tempPlu;
        if (!infile.good())
        {
            infile.clear();
            infile.ignore();
        }
        infile >> tempName;
        if (!infile.good())
        {
            infile.clear();
            infile.ignore();
        }
        infile >> tempType;
        if (!infile.good())
        {
            infile.clear();
            infile.ignore();
        }
        infile >> tempPrice;
        if (!infile.good())
        {
            infile.clear();
            infile.ignore();
        }
        infile >> tempInv;
        if (!infile.good())
        {
            infile.clear();
            infile.ignore();
        }
        if (tempType == 0){
            unit.setInventory(tempInv);
            unit.setName(tempName.c_str());
            unit.setPLU(tempPlu);
            unit.setType(tempType);
            unit.setPrice(tempPrice);
            inputU->setFruit(unit);
            unitList.appendNode(inputU);    
        }


    checkout(poundList, unitList);
    system("pause");
    return 0;
}
void checkout(linkedListU p){

int key = -10;
puItem * searchU=nullptr;
int counter = 0;
double total = 0;
double amount;
cout << "Enter the plu for the item you want or enter 0 to exit: ";
cin >> key;
while (key < 0)
{
    cout << "\nInvalid input please re enter: ";
    cin >> k
    searchU = p.pluLookup(key);
}
while (key)
{

When it gets to the plu lookup it throws the error and I cant seem to find out why. I know that that is error is for deleting something twice but I couldnt find any instance of that in this code.

  • Can you tell us what the error is? – GabrielOshiro May 02 '15 at 05:14
  • Why didn't you use `std::string name` in your `item` class instead of `char* name`? You used it in `main` so why not in `item` also? You can avoid these types of errors by not (or reducing) the need to handle dynamic memory allocation. In addition, your `puItem` class requires a correct, user-defined copy constructor and assignment operator, since you are returning it by value in the `getFruit` function. Please read up on the "rule of 3" – PaulMcKenzie May 02 '15 at 05:19
  • @GabrielOshiro The error is BLOCK_TYPE_IS_VALID(pHead->nBlockUse) it gets thrown during the plu lookup function. – Scotty May 02 '15 at 05:23
  • Running this under a debugger and putting a breakpoint on the only `delete` in your code, waiting for it to fire, and showing a backtrace of the call stack each time it does so, will probably be incredibly informative for you. It will also fully amplify the reason Paul suggests you read about the [Rule of Three](http://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)) – WhozCraig May 02 '15 at 05:43

1 Answers1

1

There are a lot of issues with your code, where most of them are due to your classes not being safely copyable (they lack a user-defined copy constructor and assignment operator, and destructor). Please see the rule of 3:

What is The Rule of Three?

Your checkout function has the following prototype:

void checkout(linkedListU p){

This means that you are passing linkedListU by value. Since linkedListU failed to follow the rule of 3, when you pass this type by value, you will invoke the compiler-defined copy constructor which only makes shallow copies, thus causing undefined behavior.

Your linked list class has members that are pointers to dynamically allocated memory, and they need to be properly handled by following the Rule of 3 at the link above. Since you failed to do that, passing by value cannot be done safely.

To address this issue, you can pass the linked list by reference, not by value:

void checkout(linkedListU& p){

This will stop the copying to occur, but it really doesn't address the underlying issue of the rule of 3 not being used in any of your classes.

For example, your puItem is being returned by value in the puItem::getFruit function, and also passed by value in the puItem::setFruit function. Calling these functions without any changes also invokes undefined behavior due to these classes not being safely copyable (also due to you using members that point to dynamically allocated memory).

To address this issue, the first thing you can do is change the base class item to use std::string name; instead of char *name;. This makes item now a copyable class without having to write user-defined copy operations or to have a destructor that needs to delete name;. The reason is that once std::string is used, all of your members in item can be copied without user intervention. The compiler default version would suffice.

class item
{
   int plu;
   std::string name;
   double price;
   double inv;

public:
    void setPLU(int g) { plu = g; }
    void setName(const char * p) { name = p; }
    void setPrice(double g) { price = g; }
    void setInventory(double g) { inv = g; }
    int getPlu() { return plu; }
    std::string getName() { return name; }
    double getPrice() { return price; }
    double getInventory() { return inv; }
};

Note that there is no need for a destructor. Now, when puItem is derived from this class, puItem is also now safely copyable, and you can pass and return puItem by value:

class puItem : public item
{
    bool type;
    public:
        void setType(bool g) { type = g; }
        bool getType() { return type; }
};

Making these changes totally eliminates usage of these classes from causing heap errors, double deletion errors, or memory leaks. Any further errors concerning memory issues are now focused on your linked list class (which uses dynamically allocated memory). So the least, the problem is narrowed down by a huge amount and is now focused.

Community
  • 1
  • 1
PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45