Opinions on this inventory Code.

Started by
7 comments, last by Endurion 18 years, 4 months ago
Hi there... I am very new to programming and have been playing around with a text adventure game as a way to improve my coding skills and learn c++. I have made a sinple "demo" of the way the inventory will handel picking up new objects (once I get it all working it will go in the charicter class with the other things) Basicaly I am using 2 vectors. 1 for sting names of items and the other for short number of items. I used vectors as they have the easy push.back and erase() functions that dynamicly resize the array/vector thing-o. Anyway, I need the program to check to see if the item exists in the inventory before it dose anything. Then if it finds the item in the inventory it just add number of the item picked up to how many of that item is in the inventory. Then after that test/function all happens if the item picked up is not in the inventory it needs to add with a push_back the item name and the number of that item picked up. in this test you pick up a 1 shield that you do not already have. The code below dose this but it feels like it is a bad way to go about it.. I use a break in a while string for example. I have tred to put a lot of comments for easy reading of what is going on for this post.
#include <iostream>
#include <string>
#include <vector>
using namespace std;


int main ()
{
	vector <short> numberItems; // Declar vector for Number of Items
	vector <string> inventory; // Declar vector for Item Names
	string item = "shield"; // Test runs as if the user has picked up a shield
	short numbItems = 1; // Test runs as if the user has picked up 1 shield only.

// START - Example of existing inventory the user may have
	inventory.push_back("napsack");
	numberItems.push_back(1);
	inventory.push_back("cape");
	numberItems.push_back(1);
	inventory.push_back("sword");
	numberItems.push_back(1);
	inventory.push_back("healing potion");
	numberItems.push_back(5);
	inventory.push_back("salve");
	numberItems.push_back(10);
	inventory.push_back("C++ Programming Book");
	numberItems.push_back(1);
// END - Example of existing inventory the user may have


// START - List Inventory before user picks up the 1xshield
	cout << "\n\nYou inventory contains:";
	cout << "\n-----------------------\n";
	if (inventory.size() != 0) // is the inventory is empty skip the next bit.
		for (int i = 0; i < inventory.size(); ++i) // loops through inventory and lists all the items
			cout << "\n" << numberItems<< "x " << inventory; // prints out the Number of items & the item names.
		else 
		cout << "\nYou have nothing in your inventory.";
	cout << "\n\n-----------------------";
// END - List Inventory before user picks up the 1xshield


// Start - This is the main bit and the point of my post
	bool test = true; // set a bool value that exists outside the search loop for IF there is already a item in the inventory of same name
	for (int search = 0; search < inventory.size(); search++) // search each element of the inventory
		 {
			if (item == inventory[search]){ // if it finds a item in inventory with same name as the item the user picked up (shield)
				cout << "\n\n\nFOUND in the Vector - " << item;
				numberItems[search] += numbItems; // add to the existing number of the item the ammount of the items picked up.
				test = false; // set the bool value to false to use in following while statment
			}
		 }
	while (test != false){ // if the bool value is not false then the program did not find the item already in inventory
			inventory.push_back(item); // add new item
			numberItems.push_back(1); // add new number of item picked up
		break; // This loop will go forever unless I run this break.
	}
	
// Print out adjusted inventory
	cout << "\n\nYou inventory contains:";
	cout << "\n-----------------------\n";
	if (inventory.size() != 0)
		for (int i = 0; i < inventory.size(); ++i)
			cout << "\n" << numberItems<< "x " << inventory;
		else 
		cout << "\nYou have nothing in your inventory.";
	cout << "\n\n-----------------------";

	cout << "\n\n\n\nPlease press enter to exit program.";
	cin.ignore(cin.rdbuf()->in_avail() + 1);
return 0;
}




--OUTPUT--

You inventory contains:
-----------------------

1x napsack
1x cape
1x sword
5x healing potion
10x salve
1x C++ Programming Book

-----------------------

You inventory contains:
-----------------------

1x napsack
1x cape
1x sword
5x healing potion
10x salve
1x C++ Programming Book
1x shield

-----------------------


Anyone care to comment on how I have gone about this? Know a better way? Please let me know. This code works but seams wierd to me like I am not doing this right.. I was under the understanding that if you code well you shouldn't have to use breaks. Thanks
There is nothing able to be imagined by one man that can not be achieved by many - Jules Verne
Advertisement
Well, if it works it can't be bad ;)

A nice idea would be to use the std::map. This way you can use one container instead of syncing two:

std::map<std::string,int> mapInventory;

// add a salve to the inventory
mapInventory["Salve"]++;

// add two vials
mapInventory["Health Vial"]++;
mapInventory["Health Vial"]++;

or

mapInventory["Health Vial"] += 2;


Minor flaw: When removing items you may want to erase an entry from the map once the count reaches zero.

Fruny: Ftagn! Ia! Ia! std::time_put_byname! Mglui naflftagn std::codecvt eY'ha-nthlei!,char,mbstate_t>

Can a map have any number of parameters or only 2?
----------------------------------------------------------Rating me down will only make me stronger.----------------------------------------------------------
Couldn't the while loop with the break statement be replaced with an if statement? Maybe I'm not understanding exactly what is going on but I don't see why the while loop is required at all.





If its not heresy to you, you could use a GOTO statement as an early exit from the search loop (you dont have to scan the rest of the list once you found a matching items -- on average you only need to search thru half of them)



for ( loop items) // loop all items
{
if (item.type = typex)
{ // found it
increment count
goto FOUND_ITEM;
}
//else next item....
}

// falls thru if not found
***push new item here ***

FOUND_ITEM: // jump here if found item in inventory....
// do whatever else follows....

@Anonymous Poster - You are completly correct... an if statment instead of the while loop will do the same thing and remove the break statment!!! Thanks a lot...

Any opinions on how I am going about the inventory in genral?
There is nothing able to be imagined by one man that can not be achieved by many - Jules Verne
I prefer things be a bit more scalable...

#include <iostream>#include <string>#include <vector>#include <algorithm>struct object{	unsigned int id;	virtual std::string type() const = 0;	virtual std::string display_name() const = 0;};struct actor;struct item : object{	actor* owner;	virtual double weight() const = 0;	virtual std::size_t stack_count() const { return 1; }};struct weapon : item{	virtual unsigned short damage_low() const = 0;	virtual unsigned short damage_high() const = 0;};struct actor : object{	std::vector< std::vector<item*> > inventory;	std::string proper_name;	virtual std::string display_name() const { return proper_name; }	void display_name( std::string const& name ) { proper_name = name; }	void print_inventory()	{		using std::cout;		cout << "Inventory for actor: " << display_name() << '\n';				for( std::vector< std::vector<item*> >::iterator slot_iter = inventory.begin(); slot_iter != inventory.end(); ++slot_iter )		{			cout << "Slot " << std::distance( inventory.begin(), slot_iter ) << " : ";			std::vector<item*>& slot = *slot_iter;			for( std::vector<item*>::iterator item_iter = slot.begin(); item_iter != slot.end(); ++item_iter )			{				item* i = *item_iter;				cout << '[' << i->display_name() << ']';			}			cout << '\n';		}	}	bool add_to_inventory( item* item_to_add )	{		for( std::size_t slot_index = 0; slot_index < inventory.size(); ++slot_index )		{			if( add_to_inventory( slot_index, item_to_add ) )			{				//was able to slip the item into this slot				return true;			}		}		//no room in inventory		return false;	}	bool add_to_inventory( std::size_t slot_index, item* item_to_add )	{		//try to insert the item at this slot		//check if this slot is beyond the bounds of the inventory		if( slot_index < inventory.size() )		{			std::vector< item* >& slot( inventory[ slot_index ] );			//if the slot is empty, add it right in			if( slot.empty() )			{				slot.push_back( item_to_add );				item_to_add->owner = this;				std::cout << "Adding item: " << item_to_add->type() << " to inventory slot " << slot_index << " as base item.\n";				return true;			}			else			{				//this slot is not empty - check if we can add the item here				item* base_item = slot[ 0 ];				//first make sure this item matches the item type that's in the slot				if( base_item->type() == item_to_add->type() )				{					//now make sure that the number of items in that slot doesn't exceed the stack_count for this item type					if( slot.size() < item_to_add->stack_count() )					{						//all good!						slot.push_back( item_to_add );						item_to_add->owner = this;						std::cout << "Stacking item: " << item_to_add->type() << " to inventory slot " << slot_index << ".\n";						return true;					}				}			}		}		return false;	}};namespace go //specific game objects are in this namespace{	struct shield : item	{		virtual std::string type() const { return "shield"; }		virtual std::string display_name() const { return "Shield"; }		virtual double weight() const { return 5.0; }	};	struct knapsack : item	{		virtual std::string type() const { return "knapsack"; }		virtual std::string display_name() const { return "Knapsack"; }		virtual double weight() const { return 3.0; }	};	struct cape : item //might inherit from 'clothing' instead	{		virtual std::string type() const { return "cape"; }		virtual std::string display_name() const { return "Cape"; }		virtual double weight() const { return 1.0; }	};	struct sword : weapon	{		virtual std::string type() const { return "sword"; }		virtual std::string display_name() const { return "Sword"; }		virtual unsigned short damage_low()  const { return 2; }		virtual unsigned short damage_high() const { return 5; }		virtual double weight() const { return 6.0; }	};	struct healing_potion : item	{		virtual std::string type() const { return "potion_healing"; }		virtual std::string display_name() const { return "Healing Potion"; }		virtual double weight() const { return 0.5; }		virtual std::size_t stack_count() const { return 5; }	};	struct human : actor	{		virtual std::string type() const { return "human"; }	};}int main(){	actor* bob = new go::human;	bob->display_name( "Bob" );	bob->inventory.resize( 10 ); //10 slots for items	item* sword = new go::sword;	std::vector<item*> potions( 10 );	{		std::size_t count = 10;		while( --count )		{			if( count == 8 )			{				bob->add_to_inventory( sword );			}			potions[count] = new go::healing_potion;			bob->add_to_inventory( potions[count] );		}	}	std::cout << "\n\n";	bob->print_inventory();	//clean up	{		std::size_t count = 10;		while( --count )		{			delete potions[count];		}	}	delete sword;	delete bob;}



Of course, where there are pointers should be some kind of smart pointers or other handle idiom.

And the actual objects would be best created through a factory...

And some kind of runtime type system to get from an object* to an actor*, or item*, etc is necessary, especially since the factory would be returning object*'s or handles to object

Designing a good Game Object system requires a good deal of patience, and lots of iterations ;)

[Edited by - RDragon1 on December 18, 2005 4:49:26 AM]
Oops... Sorry I didn't catch that this was in the Beginner's forums. I hope I didn't scare you :)
Quote:Original post by Shamino
Can a map have any number of parameters or only 2?


A map provides a key and a value. The key is used for sorting/finding. So you're set at 2 parameters if you want to call it that way. But you can of course use a struct or class for the value (for the key as well, but then you probably need to provide a < operator).

struct tItem{  int iCount;  int iPrice;  tItem() :    iCount( 0 ),    iPrice( 0 )  {  }};std::map<std::string,tItem>  mapItemsmapItems["Huge Sword of Whoopass"].iCount++;


Also, if you don't need extra fancy properties do yourself a favour and put the struct as such in the map, not a pointer. Then you don't need to do the cleanup yourself.

Fruny: Ftagn! Ia! Ia! std::time_put_byname! Mglui naflftagn std::codecvt eY'ha-nthlei!,char,mbstate_t>

This topic is closed to new replies.

Advertisement