I'm having trouble with this thread simply because this isn't a situation I could see myself getting into, at least not like the example code.

If you call a function which can cause something to go into an invalid state then common sense says you should check that it is still valid in some way. The way depends on the rest of the system; if could be as simple as the function returning a value to indicate the object is dead, to an object which is simply no longer in a valid state but still 'active' enough that it can be called on (just wont do anything).

Even your examples aren't great;
- if the outcome of 'bar' is that client #2 should be disconnected and removed from the game then this would be flagged and carried out at a latter clean up step (beause its unlike to be as simple as 'delete client[2]'). If you include that into 'bar' then 'bar' starts having to care about more things than it should (which would probably end up causing the function name to be a lie)

- Same thing applies to the 'character in a world' setup. You would perform your world update and then check to see who needs to be removed so that on your next sweep you simply no longer have it there to worry about. If something dies on frame 24579 of a game then it should be sane to keep the entity around until the end of that frame, or indeed a 'validity' sweep at the start of frame 24580 so that references can be taken care of and things generally cleaned up.

In short 'bar' shouldn't be deleting anything, 'bar' should be putting things into a state where someone else can initaliate the clean up.

Depending on the situation, you may also consider throwing and catching exceptions. Probably not in the character case but for maybe in the client case (although, I suppose a disconnect could be considered normal in which case exceptions are ugly). For example if you had a list of open files and 'Bar()' noticed that 'i' was no longer readable for an unexpected reason (user pulling a usb stick or something) you could use exceptions and remove it from the list in the catch-block.

I think you may want to consider handling object lifetimes differently. Generally with object lifetimes there's two models I use, which greatly simplify things:

1) Ownership is the first model. Ownership is very simple. Object A owns object B. Perhaps, container C owns object B. Whatever it may be, the lifetime of B is entirely contained within another object. Only that object is responsible for deleting the object. In this case, it may simply be a member and go away when the object does, may be directly owned as an element in the container (non-pointer), or may be held in a non-reference counted smart pointer (e.g.: scoped_ptr<>). This is the simplest model because once the object has an owner, it's extremely easy to determine the relationship. If the object is no longer used, you remove it and forego any further processing on this object.

2) The second possibility is shared ownership, typically using a reference-counting smart pointer, such as shared_ptr<>. The benefit here is you don't need to worry about ownership. When the object isn't being referenced anymore, it will be deleted. It simplifies your concerns. The downside is it's harder to know when it will actually get deleted, since the ownership concept is not so clear.

Your example violates the first principle, because deleting the object without notifying the array has abused the ownership principle. You can't delete the object and have the array pointing at garbage. That's an inconsistent state in your program.

Your example also violates the second principle, because you said it has the potential of being deleted while it is still being used, as it is still part of the array.

I would try to rectify your design to use one of the two mechanisms. In your case, I would probably advise an ownership mechanism such as this:

Quote:Original post by VBStrider
One example of where this becomes a problem is in network packet handling. For this example, objects are called clients and Bar() handles network packets for client #i. Among other reasons, an error in a packet could cause the client to be forcibly disconnected (and thus, the client object would be removed). The call to CClass::A() represents any code that would be executed after the client has been removed.

This is an interesting example because I have to contend with exactly this problem in some code I work with.

The proper solution - and yes, I call it proper because I strongly believe any other way is wrong - is that the packet parsing code should never be able to perform an operation that destroys the client object. That code should know nothing about clients or networks and shouldn't have any interest in the lifetime of those objects. It should have one single responsibility, which is to understand the packet. If the packet was valid, return the parsed content or a success value or both. If the packet was invalid, return an error code or raise an exception. The code above that, ie. the code that called it, will decide how to handle that situation, and that code should be the code that actually owns the client object and has authority over deleting it. After all, what if you wanted to have it so that a corrupted message is resent, or a diagnostic message is sent, or some other behaviour? This shouldn't be done by the code that finds the error, but by the code that owns the object. Give it the error, and let it decide what to do.

In your example, it should be clear that Foo() needs implicit ownership of the objects array, in order to iterate over it safely. Therefore Foo() also has ownership of the contents of the array. Bar(), therefore, has no ownership of those objects and should not be deleting anything at all. When you consider that Foo is a high level part of the app that owns Clients and Bar is just a low level packet parsing function, this makes intuitive sense.

Really, you don't want these low level functions having sporadic side-effects where sometimes they change your data and sometimes they don't. Ideally, either a function should always be performing a predictable sort of mutation on an object or it should be doing nothing at all. Keeping to this rule makes your code a lot easier to debug because you're not always having to guess at the results of each operation.

You must know who 'owns' the object at all times, and when you 'lend' that object to other functions, those functions must never be allowed to destroy the object. Instead, they must signal a wish to have it destroyed when they relinquish control.