• Advertisement
Sign in to follow this  

Neglecting container check, how bad is my teammate?

This topic is 1128 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Does anyone find this code being very bad?

Can you rate this from 0 to 10. I am very interested what is your opinion on neglacting to check containers properly before accessing them.

 

Just for a little context, they spent around 2hours trying to find this error which was causing crashes:

void Unit::LoopUnitAI(Object & o)
{
    if(stateAction == uni::BSFollowNextCommand)
    {

        if(isUpdateLogic(uni::LOGIC_UPDATE_DELAY + rand()% uni::LOGIC_UPDATE_DELAY_DISTANCE))
        {
            if(listAIActions.front().check()) //<---- Error lies here
            {
                listAIActions.pop_front();
                isActionFinished = true;
            }
            if(isActionFinished)
            {
                if(listAIActions.size() > 0)
                {
                    isActionFinished = false;
                    listAIActions.front().action();
                }
                else
                {
                    setStateAction(*this, uni::BSStandGuard);
                }
            }
        }

    }
    else if(stateAction == uni::BSStandGuard)
    {
        if(isUpdateResetLogic(uni::LOGIC_RESET_DELAY))//Every so often check if there are no new commands
            setStateAction(*this, uni::BSFollowNextCommand);
    }
}
Edited by Cl9

Share this post


Link to post
Share on other sites
Advertisement

Before I could rate this code, is there any other code prior to this which checks that listAIActions is not empty?

 

Is the function you paste documented to state that the list should not be empty when it is called?

 

To me things like this are acceptable if clearly documented as expected behaviour... 

Share this post


Link to post
Share on other sites

Before I could rate this code, is there any other code prior to this which checks that listAIActions is not empty?

 

Is the function you paste documented to state that the list should not be empty when it is called?

 

To me things like this are acceptable if clearly documented as expected behaviour... 

 

There were no checks to ensure that listAIActions wasn't empty, or it wouldn't have been crashing surely? ;)

And I doubt he classed it as expected behaviour if it took him 2 hours to find...

(Basically, it was an oversight)

Edited by Cl9

Share this post


Link to post
Share on other sites

If I could ever give multiple upvotes for a reply, I think it would be that one.

Edited by ferrous

Share this post


Link to post
Share on other sites
if(listAIActions.front().check()) //<---- Error lies here
{
   listAIActions.pop_front();
   isActionFinished = true;
}
if(listAIActions.size() > 0)
{
    isActionFinished = false;
    listAIActions.front().action();
}

The fact that code does two similar things with very different style suggest me that either those are writed by two different persons, in different times or the error one is just forget to clean/refactor. Bugs happen, get over it.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement