Need some help with OOP for a Inventory system

[SCB3]

So the task I have been given is

"Using an object orientated approach, write a program that add's , stores, deletes and can edit items from a players bank of characters, weapons, objects and spells"

So, essentially what I'm trying to do is create 4 arrays (character, weapon, object, spell) and then the menu was giving a choice of adding one of those to its respective array.

What's I'm struggling with is using pointers to do this, I've only just started using OOP and do understand using classes, constructors and decontructors fine

Heres the code I have so far, I'm trying also to keep it efficient, someone has mentioned about using only one class instead of 4, but unsure how to do that

#include<iostream>
#include<string>
using namespace std;

class cnode
{
	string name;
	string gender;
	string character;	
	cnode* cnext;
public:
	cnode()
	{
		cout<<"Please Enter Name"<<endl;
		getline(cin, name);
		cout<<"please Enter Gender"<<endl;
		getline(cin, gender);
		cout<<"Enter Character Class"<<endl;
		getline(cin, character);
		cnext=NULL;
	}

	~cnode()
	{
		cout<<"Deleting"<<endl;
	}

	void setpointercharacter(cnode* characterpoint)
	{
		cnext=characterpoint;
	}

	string getcharactername()
	{
		return character;
	}

	cnode* getcharacterpointer()//Get Character Pointer
	{
		return cnext;
	}
};

class wnode //Weapon Class
{
	string weapon;
	string uniqueweaponname;
	int magic, cost;
	wnode* wnext;
public:
	wnode()
	{
		cout<<"Enter Weapon"<<endl;
		getline(cin, weapon);
		cout<<"Enter Unique Weapon Name"<<endl;
		getline(cin, uniqueweaponname);
		cout<<"Enter Weapon Magic Resistance"<<endl;
		cin>>magic;
		cout<<"Weapon Mana Cost:"<<endl;
		cin>>cost;
		wnext=NULL;
	}
~wnode()
	{
		cout<<"Deleting Weapon"<<endl;
	}
void setpointerweapon(wnode* weaponpoint)
	{
		wnext=weaponpoint;
	}

	string getweaponname()
	{
		return weapon;
	}

	wnode* getweaponpointer()
	{
		return wnext;
	}
};

class onode //Object Class
{
	string object;	
	int magic, cost;
	onode* onext;
public:
	onode()
	{
		cout<<"Enter Object"<<endl;
		getline(cin, object);
		cout<<"Enter Object Magic Damage"<<endl;
		cin>>magic;
		cout<<"Object Mana Cost:"<<endl;
		cin>>cost;
		onext=NULL;
	}
~onode()
	{
		cout<<"Deleting Object"<<endl;
	}
void setpointerobject(onode* objectpoint)
	{
		onext=objectpoint;
	}

	string getobjectname()
	{
		return object;
	}

	onode* getobjectpointer()
	{
		return onext;
	}
};
class snode //Spell Class
{
	string spell;	
	int magic, cost;
	snode* snext;
public:
	snode()
	{
		cout<<"Enter Spell"<<endl;
		getline(cin, spell);
		cout<<"Enter Spell Magic Damage"<<endl;
		cin>>magic;
		cout<<"Spell Mana Cost:"<<endl;
		cin>>cost;
		snext=NULL;
	}
~snode()
	{
		cout<<"Deleting Spell"<<endl;
	}
void setpointerspell(snode* spellpoint)
	{
		snext=spellpoint;
	}

	string getspellname()
	{
		return spell;
	}

	snode* getspellpointer()
	{
		return snext;
	}
};

void addcharacter(cnode* start)
{
	cnode* temp;
	cnode* current;
	cnode* past;
	temp = new cnode;
	string playerchoice;

	if (start==NULL || temp->getcharactername()<start->getcharactername())
	{
		temp->setpointercharacter(start);
		start=temp;
	}
	else
	{
		current=start;
		while(current!=NULL && temp->getcharactername()>current->getcharactername())
		{
			past=current;
			current=current->getcharacterpointer();
		}
		past->setpointercharacter(temp);
		temp->setpointercharacter(current);
	}
	cout<<"Would you like to add a new Character?"<<endl;
	cin>>playerchoice;
	if(playerchoice=="yes")
	addcharacter(start);
	else if (playerchoice=="no")
		return;
}

void addweapon(wnode* wstart)
{
	wnode* temp;
	wnode* current;
	wnode* past;
	temp = new wnode;
	string playerchoice;

	if (wstart==NULL || temp->getweaponname()<wstart->getweaponname())
	{
		temp->setpointerweapon(wstart);
		wstart=temp;
	}
	else
	{
		current=wstart;
		while(current!=NULL && temp->getweaponname()>=current->getweaponname())
		{
			past=current;
			current=current->getweaponpointer();
		}
		past->setpointerweapon(temp);
		temp->setpointerweapon(current);
	}
	cout<<"Would you like to add a new Weapon?"<<endl;
	cin>>playerchoice;
	if(playerchoice=="Yes")
	addweapon(wstart);
	else if (playerchoice=="No")
		return;
}

void addobject(onode* ostart)
{
	onode* temp;
	onode* current;
	onode* past;
	temp = new onode;
	string playerchoice;

	if (ostart==NULL || temp->getobjectname()<ostart->getobjectname())
	{
		temp->setpointerobject(ostart);
		ostart=temp;
	}
	else
	{
		current=ostart;
		while(current!=NULL && temp->getobjectname()>=current->getobjectname())
		{
			past=current;
			current=current->getobjectpointer();
		}
		past->setpointerobject(temp);
		temp->setpointerobject(current);
	}
	cout<<"Would you like to add a new Object?"<<endl;
	cin>>playerchoice;
	if(playerchoice=="Yes")
	addobject(ostart);
	else if (playerchoice=="No")
		return;
}

void addspell(snode* spstart)
{
	snode* temp;
	snode* current;
	snode* past;
	temp = new snode;
	string playerchoice;

	if (spstart==NULL || temp->getspellname()<spstart->getspellname())
	{
		temp->setpointerspell(spstart);
		spstart=temp;
	}
	else
	{
		current=spstart;
		while(current!=NULL && temp->getspellname()>=current->getspellname())
		{
			past=current;
			current=current->getspellpointer();
		}
		past->setpointerspell(temp);
		temp->setpointerspell(current);
	}
	cout<<"Would you like to add a new Spell?"<<endl;
	cin>>playerchoice;
	if(playerchoice=="Yes")
	addspell(spstart);
	else if (playerchoice=="No")
		return;
}

void delcharacter(cnode* start)
{
	string csearch;
	cnode* current;
	cnode* past;

	if (start=NULL)
		cout<<"No Character"<<endl;
	else
	{
		cout<<"Character to be killed is: "<<endl;
			getline(cin, csearch);
		if (start->getcharactername()==csearch)
		{
			current=start;
			start=start->getcharacterpointer();
			delete current;
		}
		else
		{
			current=start;
			while(current!=NULL && current->getcharactername()!=csearch)
			{
				past=current;
				current=current->getcharacterpointer();
			}
			if (current=NULL)
				cout<<"Character not found"<<endl;
			else
			{
				past->setpointercharacter(current->getcharacterpointer());
				delete current;
			}
		}
	}
}

void characteredit()
{

}

void delweapon(wnode* start)
{
	string wsearch;
	wnode* current;
	wnode* past;

	if (start=NULL)
		cout<<"No Character"<<endl;
	else
	{
		cout<<"Weapon Destroyed:  "<<endl;
			getline(cin, wsearch);
		if (start->getweaponname()==wsearch)
		{
			current=start;
			start=start->getweaponpointer();
			delete current;
		}
		else
		{
			current=start;
			while(current!=NULL && current->getweaponname()!=wsearch)
			{
				past=current;
				current=current->getweaponpointer();
			}
			if (current=NULL)
				cout<<"Weapon not found"<<endl;
			else
			{
				past->setpointerweapon(current->getweaponpointer());
				delete current;
			}
		}
	}
}

void weaponedit()
{

}

void display()
{

}

void main()
{
	char option;
	cnode* start=NULL;
	wnode* wstart=NULL;
	onode* ostart=NULL;
	snode* spstart=NULL;
	
	do
	{
		cout<<endl<<endl<<endl;
		cout<<"Main Menu"<<endl<<endl;
		cout<<"1. Add Character"<<endl;
		cout<<"2. Add Weapon"<<endl;
		cout<<"3. Add Magical Object"<<endl;
		cout<<"4. Add Spell"<<endl;
		cout<<"5. Display All"<<endl;
		cout<<"Quit"<<endl;
		cin>>option;
		cin.ignore();
		switch(option)
		{
		case '1':addcharacter(start);
			break;
		case '2':addweapon(wstart);
			break;
		case '3':addobject(ostart);
			break;
		case '4':addspell(spstart);
			break;
		case '5': display();
			break;
		case 'q':
			break;
		default: cout<<"You have entered a invaild option"<<endl;
		}
	}
	while(option!='q');
}

[Mathes]

Hello,

first you should check the compiler warnings / errors. I am not able to compile the program.
main must have the return type int, and also you have a few if (x = y) instead of x == y in your code.

Also I would prefer std::list of Characters / Weapons /..., that avoids the handling of raw pointers.
Unless of course you have to use them, or want to use them for practice.

Regards,
mathes

[SCB3]

I'm aware of the errors, been trying to solve them along the way as well (it was only when I began to add weapons etc)

I have to use pointers as part of it (plus its good practice for next year, where I'll be using a lot more object orientated programming),

Here is the criteria of what I have to enter if this helps in any way

Characters:
Unique name
Sex
Type:                      wizard/witch/hero/heroine/sponsor/worker/creature
Magical ability:                    Value between 0 and 10
Strength:                 Value between 0 and 10
Health:                    Value between 0 and 10
Wealth:                   Value between 0 and 10
 
Weapon:
                        Unique name
                        Type                        Axe/sword/dagger/catapult
                        Magical resistance:  Value between 0 and 10
                        Cost                        Value between 0 and 10
 
Magical object:
                        Unique name
                        Type:                   wand/shield/invisibility cloak/transporter
                        Magical strength:  Value between 0 and 10
                        Cost                    Value between 0 and 10
 
Spell:
                        Unique name
                        Action:                  stunning/revealer/object or weapon summoning
                        Magical strength:  Value between 0 and 10
                        Cost                    Value between 0 and 10

thanks for the help btw

[Mathes]

If I fix the errors the programm runs. At least I can create a character without any problem.

So what exactly is the problem?

Btw. using pointers and OOP are two different things.

[SCB3]

Oh yea, so they are, a rookie mistake

The problem I've got its where to go from here, I'm trying to make it more efficient (without using repetitive coding) and be able to edit any of these as well, just a bit unsure of how to do that exactly

also I'm not sure what you mean by this:

Also I would prefer std::list of Characters / Weapons /..., that avoids the handling of raw pointers.
Unless of course you have to use them, or want to use them for practice.

[Mathes]

Let's take cnode as example. It currently does two things.
It holds the information of a character (name, class, gender) and it holds the information about the next character.

This can be split up. Let one class focus on the information needed for a character. And let another class take care of holding all the characters.

At the bottom of this post is a little program that uses a std::list to hold the characters.

Before you make it more efficient make it work. I have seen too much people (including me!) who start optimisation and refactoring without having anything working. If you wait until you have a working piece of software, you will notice if you break anything while refactoring / optimising.

In my optinion there will be some repetitive code, because you must perform similar actions on different objects.

Another thing: do not put std::cin in a constructor. There are situations where default objects are created (e.g. std::vector::resize), and then the user is forced to enter all the data. If you need cin to get data needed for constructing of an object, put it in a seperate function.

Ok, now here is the little peace of code, that allows you to create characters and displaying them.
Checking whether a character already exists with a given name is not implemented yet. Neither is editing.
Adding another type to bank, e.g. weapons is basically done like the characters. Add a class / struct, add a list in bank, add the functions. After that check what code is duplicated and try to merge it.

#include <list>
#include <iostream>
#include <string>

using namespace std;

//Its a struct, because it just holds data and I don't think that getters and setters are needed in this snippet.
struct character
{
    string name;
    string gender;
    string type;
};

class bank
{
  public:
    bank()
    : characters()
    {
    }

    void
    add_character()
    {
        characters.push_back(create_new_character());
    }

    void
    display()
    {
        for (list<character>::iterator it = characters.begin();
             it != characters.end();
             ++it)
        {
            cout << it->name << ", " << it->gender << ", " << it->type << endl;
        }
    }

  private:
    list<character> characters;

    character
    create_new_character()
    {
        character new_character;

        cout<<"Please Enter Name"<< endl;
        getline(cin, new_character.name);
        //Check whether a character already has this name [e.g. find_if]
        cout<<"please Enter Gender"<< endl;
        getline(cin, new_character.gender);
        cout<<"Enter Character Class"<< endl;
        getline(cin, new_character.type);

        return new_character;
    }
    
};

int
main()
{
    bank players_bank;
    char option;

    do
    {
        cout << "\n\n\n";
        cout << "Main Menu\n\n";
        cout << "1. Add Character\n";
        cout << "5. Display All\n";
        cout << "Quit" << endl;

        cin >> option;
        cin.ignore();

        switch (option)
        {
          case '1':
            players_bank.add_character();
            break;
          case '5':
            players_bank.display();
            break;
          case 'q':
            break;
          default:
            cout << "You have entered an invalid option" << endl;
            break;
        }
    }
    while (option != 'q');
}

If you are not familiar with list check out http://www.cplusplus.com/reference/list/
And note that another container (e.g. vector) is also possible. But since you will add (and delete) characters most of the time list seems to be a good choice.

Regards,
mathes

[SCB3]

wow, a lot to take in, but thanks, n ow I need to figure out how to delete and edit from here also, I assume I do this for Weapon, Object and spell as well?

[Mathes]

You can iterate through the list of characters to find the character with a certain name. Have a look at the display function how to iterate and access the character.

Yes, that's what I would do. Add the functionality of adding and displaying weapons and so on to the bank class. Then check what code can be merged because it is the same.

After that add e.g. the delete feature for everything. And again, check which code is redundant and merge.
And so on and so on.

[SCB3]

Yea that's what I prefer to do, get it all working then make it more efficient

So I'm assuming that if I display a character, I can add in a function to edit their name and stats at that point, rather than making it a separate menu option?

[Mathes]

display just shows you the code how to iterate.

I would prefer the extra option edit. Because than you do not mix different things.
But in the end that is up to you.

[SCB3]

As would I, ideally I want it to be:

Main Menu

1, Add Character, weapon, object or spell
2. delete character, weapon, object or spell
3. edit character, weapon, object or spell
4 Display Current Characters, Weapons, Objects or Spells
Q. Quit

Its putting that into code using pointers that I'm struggling with

[TheIdeasMan]

Hi,

Maybe the reason they want you to use pointers is so you can have polymorphism.

Weapons, objects and spells are all resources, so the Resource class could be a base class for those, and it will have pure virtual functions to describe an interface. The derived classes override the base class functions to specialise. Then you can have containers and functions that hold / take pointers to Resource. You then push_back or send arguments that are pointers to the derived class. The compiler does the right thing. There is no conversion of derived to base, either by the compiler or by your code.

You may also want to have multiple characters in terms of Hero (Soldiers, Magicians etc),and Enemies (Trolls, Dragons etc) So Character would be a base class for these also. To implement the search / edit functions, you might need another class which has a container of pointers to all the characters. The same deal holds here, push_back pointers to the derived classes.

Have a look in the tutorial, there is a section on polymorphism.

Here is a link you might find interesting, probably could find other stuff with google:
http://gameprogrammingpatterns.com/

[SCB3]

ok, thanks, thats kind of what I need, I'll check it out later, thanks everyone so far for your help

[SCB3]

ok, starting with the Characters, here's what I have now

#include <string>
#include <iostream>
using namespace std;

class character
{
	char name, gender, type;
	int magic, mana, health, wealth, strength;
	character *link;

public:
	void addnodes();
	void display();
};

void character::addnodes()
{
	character *temp2;

	character* startPtr = new character();
	character* current = new character();

	character *temp = new character();
		cout<<"Enter Name"<<endl;
		cin>>temp->name;
		cout<<"Enter Gender:"<<endl;
		cin>>temp->gender;
		cout<<"Enter Character Class/Type: "<<endl;
		cin>>temp->type;
		cout<<"Enter Health"<<endl;
		cin>>temp->health;
		cout<<"Enter Strength"<<endl;
		cin>>temp->strength;
		cout<<"Enter Wealth:"<<endl;
		cin>>temp->wealth;
		temp->link=NULL;

		if(startPtr==NULL)
		{
			startPtr = temp;
			current = startPtr;
		}
		else
		{
			temp2 = startPtr;

			while (temp2->link!=NULL)
				temp2=temp2->link;
				temp2->link=temp;
		}

}

void character::display()
{
	character *temp;
	character *startPtr=this;

	temp=startPtr;

	while (temp!=NULL)
	{
		cout<<temp->name<<endl;
		cout<<temp->gender<<endl;
		cout<<temp->health<<endl;
		cout<<temp->strength<<endl;
		cout<<temp->wealth<<endl;
		temp=temp->link;
	}
}

void main()
{
	character *temp2=NULL;
	int choice;
	do
	{
		cout<<"1. Add Character"<<endl;
		cout<<"2. Display All"<<endl;
		cout<<"q. Quit"<<endl;
		cin>>choice;
		cin.ignore();

		switch(choice)
		{
		case '1': addnodes(temp2);
			break;
		case '2': display();
			break;

		default: cout<<"Invalid Option"<<endl;
		}
	}while (choice!='q');
}

but, I have an error, on the Main Menu, addnodes() and display() are giving me an error, but unsure of why that is

[TheIdeasMan]

Hi,

An object is an object, don't try to have an object and a collection of them in the same class. Either create a collection in main with a std::vector say, or provide a separate class which holds the collection as a private variable.

When you have a class, provide a constructor that sets all of the member variables, don't use a separate function for this purpose. I would avoid having code in the class that collects the input data as well - that should be a separate non class function that calls the constructor to create an object.

When you write the constructor, use a member initialization list. http://en.cppreference.com/w/cpp/language/initializer_list

Once you have that happening, you can put in the pure virtual functions which will form the interface of how the derived classes are used.

main() returns an int, that is in the C++ standard.

Consider splitting your class into a header file *.hpp and implementation file *.cpp, this is much more tidy and flexible and allows reuse of your classes.

Avoid declaring multiple variables on 1 line. It will bite you one day, and it is untidy IMO.

Also avoid using new and delete, use the STL as much as you can. Otherwise investigate smart pointers. Use smart pointers rather than raw pointers. Note that one has to have pointers to do polymorphism.

but, I have an error, on the Main Menu, addnodes() and display() are giving me an error, but unsure of why that is

It doesn't matter now, but in future if you have errors post them here in full. It saves us doing "in brain" compiling, or otherwise using the Yoda-like telepathic force :+). We can compile with cpp.sh (the gear icon top right), but really that should be your job to show the errors.

Instead of your do loop, consider this:

bool Quit = false;

while (!Quit) {
   // call show menu function
   char option;
   // get input use tolower to get lowercase
   switch (option) {
        // cases
        case 'q':
           Quit = true;
            break;
        default:
           std::cout << "Bad Input, try again \n";
            break

   }

} 

Hope this helps :+)

[SCB3]

OK, its helping, I'm still having the same issue on the Menu

source.cpp(90): error C3861: 'display': identifier not found

[TheIdeasMan]

Hi,

When you have a class, you call it's functions via an object. You need an instance of the character (the object).

Call the function using the dot operator:

Object.function();

or if it's a pointer to object:

ptrObject->function();

So that works for void functions, if there is a return value, you have to assign that to another variable.

[SCB3]

class c
{

public:
	void display();	
	void addnodes();
};

so here to do void display I have to declare

c.display();

??

[TheIdeasMan]

Not quite:

Using the above code, but with a constructor:

class c
{
private:
      double data = 0.0; // same as if init list was used
public:
        c(); //default constructor, used to create an object with default data
        c(const double dataArg) // constructor taking one parameter of type double
              :  // intorduce initializer list
               data(dataArg) {}  // direct initialize member variable

	void display();	 // no definition shown
	void addnodes(); // no definition shown
};

//...
// ....

// in main()
c DefaultConstructdObject; // data is 0.0
c MyObject(10.0); // object named MyObject, of type c, calls the ctor with a double

MyObject.display(); // call member function using object 

Have a read of the tutorial: http://www.cplusplus.com/doc/tutorial/classes/

There is also reference material and articles :+)