0

I am looking to find the correct way to use this kind of code safely:

renderer->renderEntity(entityManager->getEntity("player"));

Entity EntityManager::getEntity(string entityName) 
{
    for (int index = 0; index < entityVector.size(); ++index)
    {
        if (entityVector[index].getName() == entityName)
        {
            return entityVector[index];
        }
    }
}

Where you want to get an object from a container within an instance of a class, and want to check it exists, so you aren't making a call on an object that doesn't exist.

I know I could alter the call to:

if (entityManager->getEntity("player" != nullptr)
{
    renderer->renderEntity("player");
}

I would like to avoid the double call to check the object exists. I guess this may be more a design than a syntax problem. Could I build the error checking into the getEntity() function that contains the for loop? I am not sure since the return value is Entity, so an Entity object must be returned. (same if it is a pointer, this isn't the code in my project, just a similar example).

martingrant
  • 164
  • 1
  • 12
  • 1
    How common is it that an entity of that name won't exist? You could throw an exception if this is a rare occurrence. Another option would be to take the standard library approach and return an iterator to the element, or to `entityVector.end()` if it doesn't exist. – TartanLlama Oct 07 '15 at 15:49
  • What should your method return if the entity is not present? – Lorenzo Belli Oct 07 '15 at 15:51
  • 1
    I would suggest using a `std::map` – Sebastian Hoffmann Oct 07 '15 at 15:51
  • 1
    You can create a default object that gets returned if the item doesn't exist; that object could have some benign behavior, or it could have a method that tells you it's not real, or it could throw an exception for every method. – Mark Ransom Oct 07 '15 at 15:54
  • You could change `renderEntity` to `renderEntityIfExists` and return if a bad entity is passed in – Eamonn McEvoy Oct 07 '15 at 15:54
  • Also see http://stackoverflow.com/questions/2537942/nullable-values-in-c – Mark Ransom Oct 07 '15 at 15:55
  • Another option would be `std::optional`. – TartanLlama Oct 07 '15 at 15:56
  • Lots of useful information here, thanks all! The ideal solution for my situation is from @Kenman Tsang with the Null Object pattern. TartanLlama - I am storing a lot of data in vectors and maps in my game engine: entities, textures, models, tilemaps and so on. It's a personal project so unlikely the error will occur, but I am just looking to get cleaner and safer code. – martingrant Oct 07 '15 at 17:40

3 Answers3

0

If you can change the return type: make getEntity() return a pointer, and NULL if the object is not present.

Alternatively as @ TartanLlama suggested you could return an iterator.

Other option (good only in some cases): your method could remember the entityName of the last time it has been invoked and the resulting index. When entering you first check if the current input is equal to the previous input: in that case you return the stored index without searching again.

Lorenzo Belli
  • 1,767
  • 4
  • 25
  • 46
  • That would work, but I was hoping to be able to not have to check if the object is NULL before using it, so I don't have a lot of if statements clogging the code up. Your second point is a good one. I like that, skipping the search if you are looking for an object you found very recently. Definitely something I could use for efficiency, thanks! – martingrant Oct 07 '15 at 17:32
  • Checking if a object is NULL is the same as checking if the element is present, they are the same case. I don't see how it can help unless this method findEntityByName() generate the element if it does not exists. – Lorenzo Belli Oct 07 '15 at 19:06
0

This should help. A simple null object pattern

https://en.wikipedia.org/wiki/Null_Object_pattern

class Entity
{
public:
    virtual void render()
    {
        // do somthing
    }
};

class EmptyEntity final : public Entity 
{
public:
    void render() override
    {
        //Do nothing or print log
    }
} static g_EmptyEntity;

Entity EntityManager::getEntity(string entityName) 
{
    for (int index = 0; index < entityVector.size(); ++index)
    {
        if (entityVector[index].getName() == entityName)
        {
            return entityVector[index];
        }
    }
    return g_EmptyEntity;
}
  • This is pretty much exactly what I am wanting, overriding any functions didn't occur to me. Really nice solution, I thought there would be a design pattern for it! Thank you. – martingrant Oct 07 '15 at 17:31
  • As you return by value, you got object slicing. – Jarod42 Oct 07 '15 at 17:55
  • Yes Jarod42. But as the asker dont have any more detail, so I cant include all the details. Entity& EntityManager::getEntity(string entityName) can help –  Oct 08 '15 at 14:36
  • Return by reference is correct, else `EmptyEntity::render()` is never called with `EntityManager::getEntity("NotFound").render()`. – Jarod42 Oct 09 '15 at 17:21
0

Returning pointer would be the simpler/clearer:

Entity* EntityManager::findEntityByName(const std::string& entityName) 
{
    const auto it = std::find_if(entityVector.begin(), entityVector.end(),
                                 [&](const auto& e) {
                                      return e.getName() == entityName;
                                 } );
    if (it == entityVector.end()) { // Not found.
        return nullptr;
    }
    return std::addressof(*it);
}

And then

auto* entity = entityManager->findEntityByName("player");
if (entity  != nullptr)
{
    renderer->renderEntity(entity);
}
Jarod42
  • 203,559
  • 14
  • 181
  • 302
  • What I was hoping to get is the case where I don't need to check if the object is null before I make a call using it. But there's some really nice stuff in your answer, I didn't know about std::find_if() and std::addressof(). Thanks, they will be useful in the future for me! – martingrant Oct 07 '15 at 17:30