// #pragma once is compiler-specific; here's the usual way#ifndef INVENTORY_H#define INVENTORY_H#include "Item.h"#include <vector>// Some notes on my formatting style: I tend to omit lots of things that aren't// necessary, and I really like my indentation to match scope, so I never// outdent 'public' or 'private', nor indent 'case' blocks.template <typename Item>struct InventorySlot {	Item kind;	int howMany;	InventorySlot() : Item(), howMany(0) {}};class Inventory {	// I wouldn't put the Item_ tags on the class names, but I'm not about to	// edit all your other code to reflect that :P	std::vector<InventorySlot<Item_Potion> > potions;	std::vector<InventorySlot<Item_MonsterDrop> > drops;	std::vector<InventorySlot<Item_Base> > bases;	std::vector<InventorySlot<Item_Upgrade> > upgrades;	std::vector<InventorySlot<Item_Weapon> > weapons;	std::vector<InventorySlot<Item_Armour> > armours;	public:	// An argument can be made for not adding things until you know you will	// need them. Personally I don't add things until I have some idea how to	// implement them.	// Also, we aren't going to need a destructor, since std::vector will do	// the work for us, so I got rid of that.	// Try to do as much as you can with the initializer list. It is common in	// C++ that you can in fact do everything here and leave the constructor	// body empty.	Inventory() : potions(5), drops(10), bases(10), upgrades(6), weapons(4), armours(4) {}	Inventory(int PotionSlots, int DropSlots, int BaseSlots, int UpgradeSlots, int WeaponSlots, int ArmourSlots) : potions(PotionSlots), drops(DropSlots), bases(BaseSlots), upgrades(UpgradeSlots), weapons(WeaponSlots), armours(ArmourSlots) {}	// I think the 'To's in the function names here were a bit sketchy. :)	bool AddPotion(Item_Potion Item);	bool AddPotion(Item_Potion Item, int Slot);	bool AddMonsterDrop(Item_MonsterDrop Item);	bool AddMonsterDrop(Item_MonsterDrop Item, int Slot);	bool AddBase(Item_Base Item);	bool AddBase(Item_Base Item, int Slot);	bool AddUpgrade(Item_Upgrade Item);	bool AddUpgrade(Item_Upgrade Item, int Slot);	bool AddWeapon(Item_Weapon Item);	bool AddWeapon(Item_Weapon Item, int Slot);	bool AddArmour(Item_Armour Item);	bool AddArmour(Item_Armour Item,int Slot);};#include "Inventory.h"// Aargh aargh aargh. C++ has real constants. Use them:const int POTION_STACK_LIMIT 10;const int DROP_STACK_LIMIT 20;const int BASE_STACK_LIMIT 20;const int UPGRADE_STACK_LIMIT 2;const int WEAPON_STACK_LIMIT 1;const int ARMOUR_STACK_LIMIT 1;// Passing arguments by const reference is probably a good idea here.bool Inventory::AddPotion(const Item_Potion& Item) {	// If you need a comment to explain what a variable is for, then the name is	// not good enough.	bool already_in_bag = false;	bool will_fit_in_bag = false;	bool space_in_bag = false;	// You don't need any enumeration here nor a 'situation' variable. What you're	// doing is checking conditions in order to set a value, and then checking	// the value to decide what to do. This is trivially replaced by checking	// the conditions in order to decide what to do. :)	// (Also, there is no way in the original code that 'Situation' could have	// been left invalid, and even if your coding style allows for 'impossible'	// default cases, you should handle them with an assertion instead.)	// We don't need to comment the loops; the purpose is obvious.	// And anyway, why loop twice? We can consider a slot for multiple properties	// at once...	// In fact, the whole process is much too complicated, now that I look at it.	// All we need to do is examine each slot, and see if it will accomodate the	// item. Instead of remembering *whether* there is an empty slot or a	// matched slot, and then looking for it later, we will look for the *first*	// empty slot, and the *first* slot which has room and holds the same item	// type.	// 'iterators' are objects that wrap up the process of indexing repeatedly	// into a vector. They pretend to be pointers; when we "increment" them, they	// start pointing to the next element of the vector, and when we "dereference"	// them, we get the corresponding element. We can initialize them to point	// at the beginning element with the .begin() member function of the vector,	// or to "one past the end" with .end(). 	// One-past-the-end iterators are in fact very useful: they make proper loop	// bounds that we can check with != rather than <= (for some kinds of 	// containers, '<' might not be meaningful - you might only be able to 	// compare for equality), conventionally represent "not found" elements, and	// the "distance" from begin to end equals the size of the container. It's 	// analogous to 'int array[10]; int* end = array + 10;' - you can't	// dereference that pointer validly, but it's a very meaningful concept.	typedef std::vector<InventorySlot<Item_Potion> >::iterator iterator;	iterator first_compatible_slot = potions.end();	iterator first_empty_slot = potions.end();	// The procedure is:	// For each slot:	//	If we haven't already found the first compatible slot, is this it?	//	Otherwise, if we haven't already found the first empty slot, is this it?	// Then, if we found a compatible slot, use it; else if we found an empty	// slot, use that; if that doesn't work, it just doesn't fit.	for (iterator it = potions.begin(); it != potions.end(); ++it) {		if (first_compatible_slot = potions.end() &&		    it->kind.GetID() == Item.GetID() && it->howMany < POTION_STACK_LIMIT) {			first_compatible_slot = it;		} else if (first_empty_slot == potions.end() && it->howMany == 0) {			first_empty_slot = it;		}	}	if (first_compatible_slot != potions.end()) {		first_compatible_slot->howMany += 1;		return true;	} else if (first_empty_slot != potions.end()) {		// OK, this part also REALLY needs to be fixed, but I can't do it locally.		// It is possible that all you need to do is assign the object with '='		// directly. If not, then what we'll do is write an "assignment operator"		// which makes that possible. In which case we will also need a		// "copy constructor" and destructor for the Item_Potion. But chances are,		// if we "need" them for this, you would have needed them for something		// eventually anyway, or else all kinds of strange things would start		// happening. Trust me.		Item_Potion& to_assign = first_empty_slot->kind;		to_assign.SetDesc(Item.GetDesc());		to_assign.SetDropSprite(Item.GetDropSprite());		to_assign.SetID(Item.GetID());		to_assign.SetInventorySprite(Item.GetInventorySprite());		to_assign.SetName(Item.GetName());		to_assign.SetRestoreValue(Item.GetRestoreValue());		first_empty_slot->howMany = 1;		return true;	}	// if we get here, the item does not fit in the bag.	return false;}bool Inventory::AddPotion(Item_Potion Item, int Slot) {	// We don't need to chain the if's together here because we're already	// using bail-out logic.	if (potions[Slot].kind.GetID() == Item.GetID() &&			potions[Slot].howMany < POTION_STACK_LIMIT) {		potions[Slot].howMany += 1;		return true;	}		if (potions[Slot].howMany == 0) {		Item_Potion& to_assign = potions[Slot].kind;		to_assign.SetDesc(Item.GetDesc());		to_assign.SetDropSprite(Item.GetDropSprite());		to_assign.SetID(Item.GetID());		to_assign.SetInventorySprite(Item.GetInventorySprite());		to_assign.SetName(Item.GetName());		to_assign.SetRestoreValue(Item.GetRestoreValue());		potions[Slot].howMany = 1;		return true;	}		return false;}// Similarly for other functions.// Although, the fact that we're repeating it for the other kinds of things// in turn points to *more* design problems :)