rrracheljoy
rrracheljoy

Reputation: 13

Is my C++ list iterator changing position, or am I doing something wrong?

I'm working on a rolodex and have gotten to the point where I'm able to add and search "cards" to and from a list just fine. However, after I complete a search and try to view the current "card" (at the current iterator position), it gives me something completely different. Any ideas why this might be happening (code excerpts below)?

Rolodex.h:

#ifndef _ROLODEX_H_
#define _ROLODEX_H_

#include <string>
#include <list>
#include <iterator>
#include <algorithm>

#include "Card.h"   //Card class definition

using namespace std;

class Rolodex
{
    public:
        void add(Card& card);
        Card getCurrentCard();
        bool search(const string&, const string&);
        void show(ostream&);
    private:
        list<Card> rolodex;
        list<Card>::iterator rolodexIt;
};

#endif

Rolodex.cpp:

#include <cctype>
#include <algorithm>
#include <cstring>

#include "Card.h"       //Card class definiton
#include "Rolodex.h"    //Rolodex class definition

using namespace std;

void Rolodex::add(Card& card)
{
    if (rolodex.empty())
    {
        rolodexIt = rolodex.begin();
        rolodex.insert(rolodexIt, card);
        return;
    }

    else
    {    
        rolodexIt = rolodex.begin();
        while (rolodexIt != rolodex.end())
        {
            if (!rolodexIt -> getLastName().compare(card.getLastName()) && !rolodexIt -> getFirstName().compare(card.getFirstName()))
            {
                rolodex.insert(rolodexIt, card);
                return;
            }

            else
            {
                int length;
                int roloLength = rolodexIt -> getLastName().size();
                int userLength = card.getLastName().size();

                if (roloLength < userLength)
                {
                    length = roloLength;
                }

                else
                {
                    length = userLength;
                }

                for (int i = 0; i < length; i++)
                {
                    char rolo = rolodexIt -> getLastName().at(i);
                    char user = card.getLastName().at(i);

                    if (rolo > user)
                    {
                        rolodex.insert(rolodexIt, card);
                        return;
                    }

                    else if (rolo < user)
                    {
                        break;
                    }
                }

                rolodexIt++;
            }
        }

        rolodexIt = rolodex.end();
        rolodex.insert(rolodexIt, card);
    }
}

Card Rolodex::getCurrentCard()
{
    return *rolodexIt;
}

bool Rolodex::search(const string& lastName, const string& firstName)
{
    list<Card>::iterator tempIt = rolodexIt;

    rolodexIt = rolodex.begin();

    while (rolodexIt != rolodex.end())
    {
        if (!rolodexIt -> getLastName().compare(lastName) && !rolodexIt -> getFirstName().compare(firstName))
        {
            return true;
        }

        else
        {
            rolodexIt++;
        }
    }

    cout << endl << "No exact matches found: Displaying closest match." << endl;
    rolodexIt = rolodex.begin();

    while (rolodexIt != rolodex.end())
    {        
        int length;
        int roloLength = rolodexIt -> getLastName().size();
        int userLength = lastName.size();

        if (roloLength < userLength)
        {
            length = roloLength;
        }

        else
        {
            length = userLength;
        }

        for (int i = 0; i < length; i++)
        {
            char rolo = rolodexIt -> getLastName().at(i);
            char user = lastName.at(i);

            if (rolo == user)
            {
                if (roloLength < userLength && i + 1 == roloLength)
                {
                    rolodexIt++;
                    return true;
                }

                else if (roloLength > userLength && i + 1 == userLength)
                {
                    return true;
                }
            }

            else if (rolo > user)
            {
                return true;
            }

            else if (rolo < user)
            {
                break;
            }
        }

        rolodexIt++;
    }

    rolodexIt = tempIt;
    return false;         
}

void Rolodex::show(ostream& os)
{
    list<Card>::iterator tempIt = rolodexIt;

    for (rolodexIt = rolodex.begin(); rolodexIt != rolodex.end(); rolodexIt++)
    {
        rolodexIt -> show(os);
    }

    rolodexIt = tempIt;
}

main.cpp

#include <iostream>
#include <cstring>

#include "Card.h"       //Card class definition
#include "Rolodex.h"    //Rolodex class definition

using namespace std;

void listDex(Rolodex temp)
{
    temp.show(cout);
}

void view(Rolodex temp)
{
    temp.getCurrentCard().show(cout);
}

void search(Rolodex temp)
{
    string lastName;
    string firstName;

    cout << endl << "Enter last and first names for search:" << endl;

    cout << "Last Name: ";
    cin >> lastName;

    cout << "First Name: ";
    cin >> firstName;

    if (temp.search(lastName, firstName))
    {
        view(temp);
    }
}

int quit()
{
    cout << endl << "Program ended." << endl;
    return 0;
}

int main(void)
{
    int status = 1;

    Rolodex testData;

    Card card[10];

    card[0].setFirstName("Tony");
    card[0].setLastName("Hansen");
    card[0].setOccupation("Writer");
    card[0].setAddress("12 E. St. NY, NY 33333");
    card[0].setPhoneNumber("555-9999");

    card[1].setFirstName("Jon");
    card[1].setLastName("Smyth");
    card[1].setOccupation("Computer Hardware");
    card[1].setAddress("CMU Computer Services\nPittsburgh, PA");
    card[1].setPhoneNumber("555-1324");

    for (int i = 0; i < 10; i++)
    {
        testData.add(card[i]);
    }

    listDex(testData);
    search(testData);
    view(testData);
    quit();
}

I tried to cut out the fat of the code, but it still looks pretty long... sorry about that. =/

Basically, the issue I'm having is when I call

view(testData);

and it returns something completely different from what I was just viewing with

search(testData);

Any help, advice, constructive criticism is appreciated. Thanks for your time!

Upvotes: 1

Views: 416

Answers (1)

Martin Richtarsky
Martin Richtarsky

Reputation: 679

view() and search() are getting passed the Rolodex object by value. That means a new object is generated for the first call to search(), and the copies' iterator gets modified inside search(). These modifications are not done to the testData Rolodex object which is later passed to view(), therefore it does not work as expected.

You should pass them in by reference or pointer.

Also note that you are accessing card[0..9], even though you only explicitly initialized the first two. This might still be ok if the Card constructor initializes the object properly.

Upvotes: 2

Related Questions