Bug caused by overarchitecting [modified]

[Rama Krishna Vavilala]

This is a slightly long one. There are different classes derived from Item and all Item classes have Load and Save behaviors. The ItemTypeInfo classes manages dynamic creation of Item objects.

class Item;

class ItemTypeInfo
{
public:
ItemTypeInfo()
{
m_nIndex = ++s_nIndex;
rgItemTypeInfo[m_nIndex] = this;
}

int GetIndex()
{
  return m\_nIndex;
}

virtual Item\* CreateItem() = 0;
static void LoadFromFile(char\* szFileName);

private:
static int s_nIndex;
static ItemTypeInfo s_rgItemTypeInfos[MAX_ITEMS];
};

template<typename ItemType>
class ItemTypeInfoT : ItemTypeInfo
{
public:
ItemType* CreateItem()
{
return new ItemType(this);
}
};

ItemTypeInfoT<Calculation> _calcs;
ItemTypeInfoT<Script> _scripts;
//Statically allocate more item type infos
...

class Item
{
public:
Item(ItemTypeInfo* pTypeInfo)
: m_pTypeInfo(pTypeInfo)
{
}

void Load(char* szFileName)
{
OnLoad(szFileName);
}

void Save(char* szFileName)
{
Write(szFileName, m_pTypeInfo->GetIndex());
//Write other data
//Each class has its own data
OnSave();
}

virtual void OnLoad() = 0;
virtual void OnSave() = 0;

public:
ItemTypeInfo* m_pTypeInfo;
};

void ItemTypeInfo::LoadFromFile(char* szFileName)
{
int nIndex;
Read(szFileName, &nIndex);
Item* pItem = s_rgItemTypeInfos[nIndex]->CreateItem();
pItem->Load(szFileName);
}

class Calculation : public Item;
class Script : public Item;

//All Item types override OnLoad and OnSave

Needless to say that the problem was discovered very soon and what I thought to be an awesome architecture had an extremely serious and extremely obvious flaw.

---

Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -Brian Kernighan

[Mike Dimmick]

There's no way to guarantee that your item type constructors run in the same order across compilations (the order that static objects' constructors run in is undefined), so the number allocated to each item type is unpredictable. Therefore you can end up loading a persisted object into the wrong type, and no doubt chaos ensues.

Stability. What an interesting concept. -- Chris Maunder

[Rama Krishna Vavilala]

Yes:) Pretty obvious and read that several times when reading C++ books. But yet I committed the mistake. In the actual application, the issue was far more complex and it involved network communication between processes and the problem was discovered when a different build of the dlls were on the server and the client machine.

---

Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -Brian Kernighan