1

The source code of this game is open source, so I decided to check it out. In it, I found something like:

// This ActionManager is basically a controller like in the MVC pattern.
void ActionManager::HandleQueryMessage(csString xml, Client* client)
{
    //check the two hands as a start.
    psItem* item = client->GetCharacterData()->Inventory().GetInventoryItem(PSCHARACTER_SLOT_RIGHTHAND);
    if(!item || !item->GetItemCommand().Length())
        item = client->GetCharacterData()->Inventory().GetInventoryItem(PSCHARACTER_SLOT_LEFTHAND);
}

The first line to get the item clearly violates the law of demeter. However, even if it was changed to client->GetCharacterData()->GetInventoryItem(PSCHARACTER_SLOT_RIGHTHAND);, it would still violate the law of demeter (as far as I know).

What can be done about it? or is this one of the places where LOD doesn't apply [as in my second example]?

Moving the GetInventoryItem up to the client class doesn't makes sense in my point of view since the client has nothing to do with the character.

Creating wrappers in the client class for all the xx methods the character class seems overkill.

Any thoughts?

Gam
  • 1,254
  • 1
  • 9
  • 18
  • 1
    The "Law of Demeter" is really a "Guideline of Demeter". This question is a bit too subjective for Stack Overflow because it is a matter of coding style how people would prefer to design some feature. If you want to review a high-quality open-source game code base, try Doom 3: http://fabiensanglard.net/doom3/ – Dietrich Epp Jun 20 '16 at 07:11
  • @DietrichEpp The planeshift source is server-side tho. The doom is client only. – Gam Jun 20 '16 at 07:58
  • That is incorrect. The Doom 3 source is not client only. – Dietrich Epp Jun 20 '16 at 08:28
  • @DietrichEpp then where's the project for the doom3 game server? I can't find it. – Gam Jun 20 '16 at 20:44
  • It really depends! As you may noticed the question voted for close as "too broad" and "primarily oppinion based", try to concretize. If the class's name is `LeftOrRightCharacterHolder` the answer is to move `client->GetCharacterData()->Inventory()` outside of the method. – Grim Jun 25 '16 at 07:06

1 Answers1

0

As you suggest yourself, if you want to follow LOD completely, you'd need functions like...

Item* Client::GetCharacterInventoryItem(int itemID) 
{
    return characterData->getInventoryItem(itemId);
}
/* ... */
Item* CharacterData::getInventoryItem(int itemID)
{
    return inventory->getItem(itemId)
}
/* ... */ 
Item* Inventory::getItem(int itemID)
{
    assert_valid_itemID(itemID);
    return inventory_table[itemId];
}

Is this additional indirection worth it? I don't know, that depends on the case, your personal preference, etc. As the comments indicate, the LOD should be viewed as a guideline, not really a law. On the other hand, in my personal experience, break it frequently and you will get into trouble... :)

Joris
  • 412
  • 3
  • 8