Seg Fault When Debugging

My program is rather extensive, however, whenever I go to debug my main function which utilizes my main class (which in turn uses various other classes) I get a seg fault wherever I put the break point. Here's my main file.

Code:

---

#include <cstdlib>
#include <fstream>
#include <iostream>
#include <string>
#include <sstream>
#include "CheckIn.h"
#include "Customers.h"

using namespace std;

int main()
{
    ifstream infile;
    infile.open("in.txt");
    CheckIn Hotel;

    if (infile.fail())
    {
        cout << "Cannot open file, program terminated." << endl;
        exit(1);
    }
    string lineread; //holds a line from the input file
    while(getline(infile, lineread))
    {
        string sub; //holds the individual words within lineread
        istringstream iss(lineread);
        while (iss >> sub)
        {
            if (sub == "add")
            {
                Customer person;
                string tempfirst, templast, temptype;
                int tempnights;
                iss >> tempfirst >> templast >> temptype >> tempnights;
                Hotel.add(tempfirst, templast, temptype, tempnights);

            }
        }
    }
    //used for testing purposes
    cout << Hotel.Platinum.CustArray[0].firstname << endl;
    cout << Hotel.NonElite.backPtr->person.firstname << endl;
    return 0;
}

---

Basically, wherever I put a break point, while debugging the file, I get a seg fault. However, that said I can run the program and it doesn't crash, so I am a bit confused at this behavior. If needed I can post my entire code. I am wondering if anyone knows of something within C++ that could cause this kind of behavior. Thanks, and let me know if you want my entire code.

Quote:

---

Originally Posted by Alterah

I am wondering if anyone knows of something within C++ that could cause this kind of behavior.

---

That "something within C++" is called "undefined behaviour". If your program has bugs, there is no guarantee that the bug will expose itself to you when you run your program.

Unlike other languages, bad C++ coding does not guarantee that your program will crash. The code could keep running for years without any indication there is something wrong lurking underneath the hood. Bad C++ coding will not guarantee that your program will work on your friend's computer, even if it works on your computer, or if it will work on machine 10,001 even if it works on 10,000 previous machines, etc.

In your case, probably, when you run your program, that program is loaded in memory at a different location than when you run it under another a debugging environment. That different location may expose the bug or hide it.

Quote:

---

Thanks, and let me know if you want my entire code.

---

Any runtime issue requires you to show the code that is being executed. Since no one knows what "add()" does, it requires you to show that code, and any code from those other functions we don't know. We also need to know what those headers contain that we don't know about.

In addition to code, we need to know what data you used to duplicate the problem.

Regards,

Paul McKenzie

I'm actually curious about this one, since your coding style seems fairly robust and avoids most of the pitfalls we usually see around here.

Alright, thanks for indicated that I code, relatively well. At least, I hope I can gather that from your statement. It seems the seg faults are random too. I...wonder if it is just a matter of where I am putting my breakpoints and when that piece of code actually executes (perhaps before variables are assigned values within my code).

Alright, I am using this as an input file:

Code:

---

add Nathan Ensz generic 2
add Sara Ensz rewards 3
add Josh Smith silver 1
add Jim Simmons platinum 2
add George Calnon gold 4
add Tim Brown gold 1
done
done
done
print
done

---

I've decided to link to the .pdf file for this lab. Basically I am implementing the check in process for a Mariott hotel. I have 5 different types of customers in order from least importance to greatest importance:
Generic
Rewards
Platinum
Silver
Gold

Rewards and up earn points. Gold customers get served first, then silver, etc. I am supposed to serve the Gold customers in a LIFO or stack manner. The rewards and generic customers are FIFO.

Heres the link: http://people.eecs.ku.edu/~mzeets/EECS268/Labs/Lab08_WriteUp.pdf



Customers.h

Code:

---

/** @Customers.h
 *  Header file for Marriot customers. Utilizes three classes.
 *  @author Greg Matthies
 *  @date 15 April 2010
 *  @version 1.0
*/
#include <cstdlib>
#include <fstream>
#include <iostream>
#include <string>

#ifndef CUSTOMERS_H
#define CUSTOMERS_H

class Customer
{
public:
    Customer();
    Customer(std::string first, std::string last, std::string type, int nights);
    int earnedPoints();
    std::string firstname;
    std::string lastname;
    std::string customertype;
    int rewardsPoints;
    int num_of_nights;
};

class RewardsCustomer : public Customer
{
public:
    RewardsCustomer();
    RewardsCustomer(std::string first, std::string last, std::string type, int nights);
    virtual int earnedPoints();
    int rewardsPoints;
};

class EliteCustomer : public Customer
{
public:
    EliteCustomer();
    EliteCustomer(std::string first, std::string last, std::string type, int nights);
    virtual int earnedPoints();
    int rewardsPoints;
};

#endif //CUSTOMERS_H

---

Customers.cpp

Code:

---

/** @Customers.h
 *  Implementation file for Marriot customers.
 *  @author Greg Matthies
 *  @date 15 April 2010
 *  @version 1.0
*/
#include <cstdlib>
#include <fstream>
#include <iostream>
#include <string>
#include "Customers.h"

using namespace std;


static const int GOLD = 3;
static const int SILVER = 2;
static const int PLATINUM = 1;

//BEGIN GENERIC CUSTOMER IMPLEMENTATION

Customer::Customer()
{
}

Customer::Customer(string first, string last, string type, int nights)
{
    firstname = first;
    lastname = last;
    customertype = type;
    num_of_nights = nights;
}

int Customer::earnedPoints()
{
    return 0;
}


//END GENERIC CUSTOMER IMPLEMENTATION


RewardsCustomer::RewardsCustomer() : rewardsPoints(0)
{
}

RewardsCustomer::RewardsCustomer(string first, string last, string type, int nights)
{
    firstname = first;
    lastname = last;
    customertype = type;
    num_of_nights = nights;
    rewardsPoints = 500;
}

int RewardsCustomer::earnedPoints()
{
    return (num_of_nights * rewardsPoints);
}

EliteCustomer::EliteCustomer()
{
}

EliteCustomer::EliteCustomer(string first, string last, string type, int nights)
{
    firstname = first;
    lastname = last;
    customertype = type;
    num_of_nights = nights;
    rewardsPoints = 1000;
}

int EliteCustomer::earnedPoints()
{
    if (customertype == "platinum")
        return (num_of_nights * rewardsPoints * PLATINUM);
    else if (customertype == "silver")
        return (num_of_nights * rewardsPoints * SILVER);
    else if (customertype == "gold")
        return (num_of_nights * rewardsPoints * GOLD);
}

---

CheckIn.h (I am still working on this one)

Code:

---

/** @CheckIn.h
 *  Header file for CheckIn Class. Utilizes various subclasses.
 *  @author Greg Matthies
 *  @date 15 April 2010
 *  @version 1.0
*/
#include <cstdlib>
#include <fstream>
#include <iostream>
#include <string>
#include "Customers.h"

#ifndef CHECKIN_H
#define CHECKIN_H

const int PLATMAXSIZE = 15; //Max size for PlatinumStack used in CheckIn class
const int SILVMAXSIZE = 10; //Max size for SilverStack used in CheckIn class
const int GOLDMAXSIZE = 5; //Max size for GoldStack used in CheckIn class

class CheckIn
{
public:
    CheckIn();
    void add(std::string firstname, std::string lastname, std::string type, int nightsstaying);
    void done();
    void print();

    class NonEliteQueue //Utilize pointer based implementation as max size is not known
    {
        public:
            NonEliteQueue();
            NonEliteQueue(const NonEliteQueue &Queue);
            ~NonEliteQueue();
            bool isEmpty();
            void enqueue(Customer &person);
            void dequeue();
            void dequeue(Customer &queueFront);
            void getFront(Customer &queueFront);
            int getSize();
        public:
            struct QueueNode
            {
                Customer person;
                QueueNode *next;
            };
            QueueNode *frontPtr;
            QueueNode *backPtr;
            int size;
    };

    class PlatinumStack //Utilize array based implementation as max size is 15
    {
        public:
            PlatinumStack();
            bool isEmpty();
            void push(EliteCustomer &person);
            void pop();
            void pop(EliteCustomer &person);
            void getName();
            void getTop(EliteCustomer &person);
            void getSize();
        public:
            EliteCustomer CustArray[PLATMAXSIZE];
            int size;
            int top;
    };

    class SilverStack : public PlatinumStack //Utilize array based implementation as max size is 10
    {
        public:
            SilverStack();
            void push(EliteCustomer &person);
            void getName();
        protected:
            EliteCustomer CustArray[SILVMAXSIZE];
    };

    class GoldStack : public PlatinumStack//Utilize array based implementation as max size is 5
    {
        public:
            GoldStack();
            void push(EliteCustomer &person);
            int GoldStack::getPoints();
            std::string getName();

        protected:
            EliteCustomer CustArray[GOLDMAXSIZE];
    };

public:
    NonEliteQueue NonElite;
    PlatinumStack Platinum;
    SilverStack Silver;
    GoldStack Gold;
};
#endif //CHECKIN_H

---

CheckIn.cpp

Code:

---

/** @CheckIn.cpp
 *  Driver program for the CheckIn class
 *  @author Greg Matthies
 *  @date 15 April 2010
 *  @version 1.0
*/
#include <cstdlib>
#include <fstream>
#include <iostream>
#include <string>
#include "CheckIn.h"
using namespace std;

CheckIn::CheckIn()
{
}

void CheckIn::add(string firstname, string lastname, string type, int num_of_nights)
{
    if ((type == "generic") || (type == "rewards"))
    {
        if (type == "generic")
        {
            Customer person(firstname, lastname, type, num_of_nights);
            NonElite.enqueue(person);
        }
        else if (type == "rewards")
        {
            RewardsCustomer person(firstname, lastname, type, num_of_nights);
            NonElite.enqueue(person);
        }
    }
    else if (type == "platinum")
    {
        EliteCustomer person(firstname, lastname, type, num_of_nights);
        Platinum.push(person);
    }
    else if (type == "silver")
    {
        EliteCustomer person(firstname, lastname, type, num_of_nights);
        Silver.push(person);
    }
    else if (type == "gold")
    {
        EliteCustomer person(firstname, lastname, type, num_of_nights);
        Gold.push(person);
    }
    cout << "Added " << firstname << " " << lastname << " " << type << " " << num_of_nights << endl;
}

void CheckIn::done()
{
    if (!Gold.isEmpty())
    {
        Gold.getPoints();
        cout << Gold.getName() << " is now being served." << endl
            << Gold.getName() << " has earned " << Gold.getPoints() << " points!" << endl;
        Gold.pop();
    }
}


//--------------------------------------------------------------
//BEGIN NONELITE QUEUE IMPLEMENTATION
//--------------------------------------------------------------

CheckIn::NonEliteQueue::NonEliteQueue()
{
    size = 0;
    frontPtr = NULL;
    backPtr = NULL;
}
CheckIn::NonEliteQueue::NonEliteQueue(const NonEliteQueue &Queue)
{
    size = Queue.size;
    if (size == 0)
    {
        frontPtr = NULL;
        backPtr = NULL;
    }
    else //not empty
    {
        //Copy first node
        frontPtr = new QueueNode;
        assert(frontPtr != NULL);
        frontPtr->person = Queue.frontPtr->person;
        QueueNode *newPtr = Queue.frontPtr;
        //Copy rest of queue
        for(QueueNode *origPtr = Queue.frontPtr->next; origPtr != NULL; origPtr = origPtr->next)
        {
            newPtr->next = new QueueNode;
            assert(newPtr->next != NULL);
            newPtr = newPtr->next;
            newPtr->person = origPtr->person;
        }
        backPtr = newPtr;
        newPtr->next = NULL;
    }
}
CheckIn::NonEliteQueue::~NonEliteQueue()
{
    while(!isEmpty())
        dequeue();
    assert((backPtr == NULL) && (frontPtr == NULL));
}
bool CheckIn::NonEliteQueue::isEmpty()
{
    return (size == 0);
}
void CheckIn::NonEliteQueue::enqueue(Customer &person)
{
    QueueNode *newPtr = new QueueNode;
    if(newPtr == NULL)
        cout << "QueueException: enqueue cannot allocate memory." << endl;
        //replace cout with throw
    else //memory allocation successful
    {
        newPtr->person = person;
        newPtr->next = NULL;
        //insert new node into queue
        if(isEmpty())//Queue is empty
            frontPtr=newPtr;
        else //Queue is not empty
            backPtr->next = newPtr;
        backPtr = newPtr; //new node is at back position
        newPtr->next = backPtr;
    }
    ++size;
}
void CheckIn::NonEliteQueue::dequeue()
{
    if(isEmpty())
        cout << "QueueException: queue empty, cannot dequeue." << endl;
        //replace with throw
    else //not empty, proceed with dequeue
    {
        QueueNode *tempPtr = frontPtr;
        if(frontPtr == backPtr) //One node in queue
        {
            frontPtr = NULL;
            backPtr = NULL;
        }
        else
            frontPtr = frontPtr->next;
        tempPtr->next = NULL;
        delete tempPtr;
    }
    --size;
}
void CheckIn::NonEliteQueue::dequeue(Customer &queueFront)
{
    if(isEmpty())
        cout << "QueueException: queue empty, cannot dequeue." << endl;
        //replace with throw
    else //not empty, retrieve front item
    {
        queueFront = frontPtr->person;
        dequeue();
    }
    --size;
}
void CheckIn::NonEliteQueue::getFront(Customer &queueFront)
{
    if(isEmpty())
        cout << "QueueException: queue empty, cannot get front item." << endl;
        //replace with throw
    else //not empty, proceed with getFront
        queueFront = frontPtr->person;
}
int CheckIn::NonEliteQueue::getSize()
{
    return size;
}
//--------------------------------------------------------------
//END NONELITE QUEUE IMPLEMENTATION
//--------------------------------------------------------------
//--------------------------------------------------------------
//BEGIN PLATINUM STACK IMPLEMENTATION
//--------------------------------------------------------------
CheckIn::PlatinumStack::PlatinumStack() : top(-1), size(0)
{
}
bool CheckIn::PlatinumStack::isEmpty()
{
    return (size == 0);
}
void CheckIn::PlatinumStack::push(EliteCustomer &person)
{
    if(top >= PLATMAXSIZE) //stack full
        cout << "StackException: stack full on push." << endl;
        //replace with throw
    else //stack not full
    {
        ++top;
        CustArray[top] = person;
        ++size;
    }
}
void CheckIn::PlatinumStack::pop()
{
    if(isEmpty()) //Stack is empty
        cout << "StackException: stack empty on pop." << endl;
        //replace with throw
    else //not empty
    {
        --top;
        --size;
    }
}
void CheckIn::PlatinumStack::pop(EliteCustomer &stackTop)
{
    if(isEmpty()) //Stack is empty
        cout << "StackException: stack empty on pop." << endl;
        //replace with throw
    else //not empty
    {
        stackTop = CustArray[top];
        --top;
        --size;
    }
}
void CheckIn::PlatinumStack::getTop(EliteCustomer &stackTop)
{
    if(isEmpty()) //Stack is empty
        cout << "StackException: stack empty on getTop." << endl;
        //replace with throw
    else //not empty
    stackTop = CustArray[top];
}
void CheckIn::PlatinumStack::getSize()
{
    cout << size;
}
//--------------------------------------------------------------
//END PLATINUM STACK IMPLEMENTATION
//--------------------------------------------------------------
//--------------------------------------------------------------
//BEGIN SILVER STACK IMPLEMENTATION
//--------------------------------------------------------------

CheckIn::SilverStack::SilverStack() : CheckIn::PlatinumStack()
{
}
void CheckIn::SilverStack::push(EliteCustomer &person)
{
    if(top >= SILVMAXSIZE) //stack full
        cout << "StackException: stack full on push." << endl;
        //replace with throw
    else //stack not full
    {
        ++top;
        CustArray[top] = person;
        ++size;
    }
}
//--------------------------------------------------------------
//END SILVER STACK IMPLEMENTATION
//--------------------------------------------------------------
//--------------------------------------------------------------
//BEGIN GOLD STACK IMPLEMENTATION
//--------------------------------------------------------------
CheckIn::GoldStack::GoldStack() : CheckIn::PlatinumStack()
{
}
void CheckIn::GoldStack::push(EliteCustomer &person)
{
    if(top >= GOLDMAXSIZE) //stack full
        cout << "StackException: stack full on push." << endl;
        //replace with throw
    else //stack not full
    {
        ++top;
        CustArray[top] = person;
        ++size;
    }
}

string CheckIn::GoldStack::getName()
{
    string name = CustArray[top].firstname + " " + CustArray[top].lastname;
    return (name);
}

int CheckIn::GoldStack::getPoints()
{
    int points = CustArray[top].earnedPoints();
    return points;
}
//--------------------------------------------------------------
//END GOLD STACK IMPLEMENTATION
//--------------------------------------------------------------

---

Client.cpp

Code:

---

/** @Client.cpp
 *  Driver program for the CheckIn class
 *  @author Greg Matthies
 *  @date 15 April 2010
 *  @version 1.0
*/

#include <cstdlib>
#include <fstream>
#include <iostream>
#include <string>
#include <sstream>
#include "CheckIn.h"
#include "Customers.h"

using namespace std;

int main()
{
    ifstream infile;
    infile.open("in.txt");
    CheckIn Hotel;

    if (infile.fail())
    {
        cout << "Cannot open file, program terminated." << endl;
        exit(1);
    }
    string lineread; //holds a line from the input file
    while(getline(infile, lineread))
    {
        string sub; //holds the individual words within lineread
        istringstream iss(lineread);
        while (iss >> sub)
        {
            if (sub == "add")
            {
                Customer person;
                string tempfirst, templast, temptype;
                int tempnights;
                iss >> tempfirst >> templast >> temptype >> tempnights;
                Hotel.add(tempfirst, templast, temptype, tempnights);

            }
            else if (sub == "done")
            {
                Hotel.done();
            }
        }
    }

    return 0;
}

---

So, I still have to write a basic exception class. And work on my done() function as well as write a print() function. Thanks for the assistance as always. Both of you have aided my learning process over the past year.

Things I've noticed so far.....

1) Customer is used as a base class, but you have not declared it to have a virtual destructor. This could be a problem.
2) You seem to be declaring virtual functions in the classes derived from Customer, yet the corresponding function is not declared virtual in Customer.
3) The rewardsPoints field is duplicated in both Customer and the classes deriving from it. While this is legal, it probably isn't what you want; unless you explicitly refer to Customer::rewardsPoints, the derived classes will be unable to access the version of the field from the base class.
4) Trying to write your own queue and stack implementations is a very likely source of trouble, although I haven't gone through your code to say whether it's right or not. I strongly recommend just using std::queue and std::stack for those purposes.

Ok, I will try to answer your statements:

1) Well we just went over inheritance, so it is a new concept. I thought that since I am using static memory, I wouldn't need to deal with destructors in this case. Does the compiler not generate one?
2) Back to point 1, new concept, didn't realize it needed to be virtual in the base clase.
3) rewardsPoints is to hold the values of 0, 500, and 1000. That's why I declared it in the derived class. Actually, I just realized what I could do, It's been changed. It no longer is declared in the derived class. I just set the value accordingly in the constructors for the derived class.
4) And, for the stack and queue, it might be the problem, but I am used to doing it like that. I've double checked everything with code from the book. Quite frankly, I wish I could use vectors, but, they don't allow that. And based off of the directions, it seems they want us to write our own stack and queue.

Edit: I am having difficulty getting the following to work:

Code:

---

int main()
{
    ifstream infile;
    infile.open("in.txt");
    CheckIn Hotel;

    if (infile.fail())
    {
        cout << "Cannot open file, program terminated." << endl;
        exit(1);
    }
    string lineread; //holds a line from the input file
    while(getline(infile, lineread))
    {
        string sub; //holds the individual words within lineread
        stringstream ss(lineread);
        while (ss >> sub)
        {
            //Trying to read ahead, if there is an add after done, then don't process done
            if (sub == "done")
            {
                infile >> sub;
                if (sub == "add")
                {
                    cout << "Done error: cannot process done command." << endl;
                    getline(infile, lineread);
                }
                else
                    Hotel.done();
                ss.str("");
                ss << lineread; //line read at this point should have a customer's info
            }
            if (sub == "add")
            {
                Customer person;
                string tempfirst, templast, temptype;
                int tempnights;
                ss >> tempfirst >> templast >> temptype >> tempnights;
                Hotel.add(tempfirst, templast, temptype, tempnights);
            }

        }
    }

    return 0;
}

---

I want to read in the file. If I get to a done command, I want to check if there is an add command ahead of it. If so, I output an error message and ignore the done command.

Quote:

---

Originally Posted by Alterah

Ok, I will try to answer your statements:

1) Well we just went over inheritance, so it is a new concept. I thought that since I am using static memory, I wouldn't need to deal with destructors in this case. Does the compiler not generate one?

---

It does, but you still have to declare it is a virtual. All you have to do is declare it, and provide an empty implementation (or you will get a link error).

Like this

Code:

---

.h
class yourclass
{
    virtual ~yourclass();
}

.cpp
yourclass::~yourclass(){}

---

The background work is still generated by the compiler, so don't go deleting stuff you otherwise wouldn't.

Quote:

---

Originally Posted by Alterah

4) And, for the stack and queue, it might be the problem, but I am used to doing it like that. I've double checked everything with code from the book. Quite frankly, I wish I could use vectors, but, they don't allow that. And based off of the directions, it seems they want us to write our own stack and queue.

---

No offense, but that's just ****ing stupid. Knowing when and how to use somebody else's code, and just plain learning about standard components is some of the most important things you should be learning. Seems teachers are gung ho about you using arrays, when professional programmers avoid them like the plague.

I took the liberty of quoting your lab:

Quote:

---

You will need to select a Stack and a Queue implementation to manage subsets of customers checking in. The check-in procedure rules should make it clear which type of implementation is
best for each. Notice, for example, that no limit is placed on how many non-elite customers there might be. On any given day, there may be ten or hundreds. Pick a reasonable implementation for
the class managing each set of customers, and justify your choices in your internal documentation.
You are encouraged to actually implement the Stack and Queue ADTs yourself, referring to those in the book only if and when you get stuck. Some variant of singly or doubly linked lists is an
obvious choice, but an array based implementation could also be used and might be preferable in some situations.

---

While it clearly says you are encourage to write them yourself, it also says you are to select the implementation, justify your choices and document it.

If your teacher is not a close minded "do it the way I said" person, I think the best thing you can do is select to use std queue/stack, explain you are using an internal array/vector or linked list implementation, and justify your choice. Both the internal container, but also the fact that those are standard and robust components, that have been thoroughly tested by millions of people for over a decade.

WORST case scenario, write your own stack queue to humor your teacher (if writing one is really part of the exercise), and then, after having written it, explain that std queue/stack is better and use that.

My 2 cents.

Quote:

---

Originally Posted by Alterah

4) And, for the stack and queue, it might be the problem, but I am used to doing it like that. I've double checked everything with code from the book.

---

That is not how you "dpuble check" things. Even if you lifted this code from a book and you checked everything from the book, dotted every "i" and crossed every "t", you still must run tests to ensure that the code is correct. The queue and stack class you wrote is the entire backbone of the program -- if it fails then the whole program fails. If this were another industry, say the construction industry, would you put together a building using untested rivets that will hold the steel beams together?

The first thing you should have done is to test thoroughly the queue class. You test it by doing random pushes, pops, etc. to see if the structure behaves correctly. You don't write a class such as this, write the rest of the program, and hope the whole thing works, all in one go.

As others have stated, there already are such classes in the standard C++ library that you could have used. Other than that, when you write your own data structure, you test your data structure before using it in a larger program. Remove the main() program you have now -- this is what the main() program could have started out with:

Code:

---

#include <cstdlib>
#include <ctime>
#include <string>
#include <sstream>

// Other includes...

using namespace std;

int main ()
{
    CheckIn Hotel;

    const char *types[] = {"generic", "rewards", "platinum", "silver", "gold"};
    srand ( time(NULL) );

    string type;   
    // add 10 names (the customer name doesn't matter
    for (int i = 0; i < 10; ++i )
    {
        string typeToUse = types[rand() &#37; 5];
        int numDays = rand() % 14 + 1;  // max two week stay
        ostringstream ostr;
        ostr << "Joe_" << typeToUse << "_" << numDays <<ends;
        Hotel.add( ostr.str().c_str(), "Test", typeToUse, numDays);
    }
    return 0;
}

---

That's it. This is how you randomly test data for your class. A random name, a randomly chosen types, and a random number of days. Every time you run this test, the data will be different.

Also:

Code:

---

void CheckIn::add(string firstname, string lastname, string type, int num_of_nights)

---

You should be passing these strings by const reference, not by value:

Code:

---

void CheckIn::add(const string& firstname,  const string& lastname, const string& type, int num_of_nights)

---

The reason is that when you pass by value, you are making a temporary copy of the entire string. Instead of this, pass the string by reference or const reference if there is not going to be a change in the string.

Regards,

Paul McKenzie

Quote:

---

Originally Posted by Paul McKenzie

As others have stated, there already are such classes in the standard C++ library that you could have used.

---

Some how, I knew you'd come along shortly to comment on this :)

Good point Paul. As for me writing my own stack an queue. I've noticed many people point out that I should have used standard C++ Library. I can agree with that, but when you've spent time writing all the code for each assignment and learning it that way, I suppose it is hard to think otherwise. Also, the labs, while not necessarily saying outright: "write your own stack and queue" seem to imply it.

Paul, you're saying I should test my code with that snippet you have above. I decided to run my program with your code for main over 50 times. Thus far the output is as expected. It did not crash. Is there a set number or a rule of thumb when testing a data structure in this manner?

Also, you've never attacked my coding style necessarily, but Lindley indicated I do my best to make my code robust. Is your opinion similar? I know there are plenty of things I can improve on, and you never seem to address my coding style. I think that's because you are trying to drill in certain points, such as the standard library and testing my data structure beforehand.

I want to make sure that while understand all the concepts, my coding is consistent with the norm. Thanks.

Monarch, thanks regarding the virtual function explanation. I know in the past assignments they have explicitly stated... do not use vectors. Yet, those assignments would have been better off with them. I will try and get it to work with my own implementation. But, over the summer break I think I will rewrite the code. That gives me to time to read up and learn how to use the standard containers better.

Quote:

---

Originally Posted by Alterah

Also, you've never attacked my coding style necessarily, but Lindley indicated I do my best to make my code robust.

---

I was referring to the fact that you use std::string rather than C-style strings, you properly put the input statements in conditions to check for failure, etc. Basically you weren't making any of the most common mistakes we see around here. The fact that you were rolling your own queues/stacks elsewhere actually goes against that conclusion, although if it's required there's not much you can do about that.

If you leverage the STL whenever possible, don't mix C coding with C++ coding carelessly, and try to avoid raw pointers, it's nearly impossible to get a segfault. Other, more obvious errors, sure, but generally not segfaults.

Quote:

---

Originally Posted by Alterah

Do I then, need to run my program multiple times (if so, how many would be good?).

---

Just once. HOWEVER, said program should thoroughly test every aspect of your implementation.

Start by pushing a single object, see if you can read it, pop it. Insert two objects, see if back reads the correct one. pop it. see if back reads the object that is left inside. Then try to insert a new object.

Check you don't leak on destruction.
Check you have proper copy construction behavior.
Check you have proper assign to self behavior.
Check empty works correctly.
etc.
etc.
etc.

As you can see, just checking is usually an even bigger task than writing the dang code. It is the most vital part of writing code though. This is the reason we can't insist enough when we tell people to use standard containers.

If you're on a Linux system, you probably have access to valgrind. That's also a really good tool for finding problematic code. I suggest you run your tests with the program running through valgrind.

Ok, I see what you mean Lindley. I am wondering if it was CodeBlocks being fussy reporting the segfaults. I do have access to a Linux machine by the way. I run both Ubuntu and Windows on my home computer. I get your point monarch. If a standard container can do the job, then I might as well use it. It's been tested over and over again. I definitely plan to rewrite my code in the summer. I am past due on the assignment, so I'd like to wrap up the assignment and submit it. Once this semester is over, I will have free time in the summer to redo it necessarily and work with the standard containers.

Code::Blocks is pretty good with segfaults, I use it exclusively on Windows and Linux.

When you build and debug your code, are you sure that none of the optimizations are turned on? That can cause segfaults with the debugger. The debugger expects nothing to be optimized, and if it is, then it might cause problems. Not only your code, but also statically linked libraries, some aren't debugger safe. *cough* wxWidgets *cough* as well as others.

Actually, I am wondering if I was being "dumb". I was trying to view the stringstream in the watches window. And, I think that could have been causing it. On a side note, I am having issues with my Queue. For whatever reason when I call getPoints() on a customer who should be a RewardsCustomer, it calls the Customer getPoints() and returns 0.

If you haven't fixed which functions are virtual and which aren't, that might happen.

I am at work right now, and don't have my most up to date code, but I am 95% certain I fixed that.

Because Customer is the base class I put a virtual in front of the function:
virtual int earnedPoints()
Then for my derived classes RewardsCustomer and EliteCustomer I've got:
virtual int earnedPoints()
for both.

For this does the virtual just go in the .h file? Or do I need to carry it over to the .cpp file as well?

The functions are set up correctly too. So, then I am safe to assume that it isn't my queue getting mixed up? As it is now I only use Customer in my queue. Because RewardsCustomer is a Customer as well, I assumed that would be good, and what you are saying is exactly that correct? Thanks again.

Edit: Eh...looks like I made a mistake. I need the virtual only the base class. I have it for the derived classes as well.

Putting virtual in the derived class shouldn't hurt anything, although you only need it in the base class.

Now, the fact that you're storing Customer objects directly in the queue may well be a problem. Polymorphism only works when you're using pointers and references; assigning a Derived object to a Base object will "slice off" the part of the object specific to the Derived class.

What you need to do is store a Customer pointer in the queue/stack. However, naturally this comes with a caveat: the thing it's pointing at will need to either exist elsewhere, or else will need to be allocated using new, which in turn means you need to make sure it gets deleted in every case.

The easiest way to make this change without a lot of extra debugging work is to store a smart pointer in the data structure rather than a raw pointer.

If you have access to TR1 or Boost, then the shared_ptr from one of those packages is a good option. However, if you don't.....in this particular case, the std::auto_ptr may be an acceptable option. You need to be careful with those because they have not-entirely-intuitive semantics. Just don't try to copy the auto_ptr object, and you should be fine.

Quote:

---

Originally Posted by Lindley

Putting virtual in the derived class shouldn't hurt anything, although you only need it in the base class.

Now, the fact that you're storing Customer objects directly in the queue may well be a problem. Polymorphism only works when you're using pointers and references; assigning a Derived object to a Base object will "slice off" the part of the object specific to the Derived class.

What you need to do is store a Customer pointer in the queue/stack. However, naturally this comes with a caveat: the thing it's pointing at will need to either exist elsewhere, or else will need to be allocated using new, which in turn means you need to make sure it gets deleted in every case.

The easiest way to make this change without a lot of extra debugging work is to store a smart pointer in the data structure rather than a raw pointer.

If you have access to TR1 or Boost, then the shared_ptr from one of those packages is a good option. However, if you don't.....in this particular case, the std::auto_ptr may be an acceptable option. You need to be careful with those because they have not-entirely-intuitive semantics. Just don't try to copy the auto_ptr object, and you should be fine.

---

I'm pretty sure there is a rule that says to NEVER use std::auto_ptr inside stl containers.

This is because to work correctly, the containers expect their elements to be assignable, which is not the case with auto_ptr. Your program will break in subtle and messy ways.

Apparently, this is so dangerous, that the stl took special measures to even prevent compiling containers of auto_ptr. How?

Quote:

---

The copy constructor and the copy assignment operator of auto_ptr take a reference to a non-const right hand side objects and the standard containers' insert() takes a reference to a const and hence auto_ptr cannot be used. It will just not allow you to create a copy of auto_ptr inside the vector (standard containers) members. That will cause you compilation errors on a compliant compiler. Nice solution..isnt it?

---

The solution I personally like is to create lists, deques or sets of objects (not vectors), and simply have them manage the lifetime of the pointed to objects.

EDIT: Whoops, apparently you never actually said to put the auto_ptr inside the vector. My bad. Still, auto_ptr were mostly meant for things simple pimpl implementation. For building objects on the heap rather than the stack. Not to be moved around that much. They are just plain too dangerous.

I completely agree. However, the queue implementation in question here is based on a linked list; one assumes that a QueueNode will never be copied (and this can be easily enforced by declaring the cctor/operator= as private). And the stack implementation is based on an array, but due to the semantics of a stack, it still ought to be possible to use auto_ptr safely.

So it seems my options are use a smart pointer or I try to overload the operators for Customer and RewardsCustomer.

I'm not sure what overloading operators has to do with it. Smart pointers are just a convenience, really. You can use raw pointers if you want.

I'll have to reread this. I guess I don't quite understand what is actually happening here. I was refering to the overloading to have both Customers and RewardsCustomers directly stored in the Queue, to try and avoid the issue. So, frontPtr->person could be a Customer and frontPtr->next->person could directly be a RewardsCustomer (I thought that's what I was doing).

Yes, that's possible, but you'd need to store both in a Customer* (or some variety of smart_ptr<Customer>). Polymorphism only works through references and pointers, remember.

There actually is an alternative, now that I think about it, at least for the queue: You could templatize QueueNode, and then give all the various template types a common base class which is the type of the next and prev pointers. That might actually be simpler in some ways, but I'm guessing the additional concepts necessary to understand it would be a road block for you at this point.

Aye. Well, I will work on the auto_ptr method. See if I can get that work. Seems like the most straightforward to understand.