Now, imagine that elsewhere I have a list of Vehicles. I want Vehicle to survive if Engine gets destructed for some reason, but I don't want Engine to survive Vehicle's destruction (particularly because it will then have a dangling pointer to Vehicle). To me, what I am doing seems safe. I don't have to worry about calling delete on "vehicle_", because whenever the shared_ptr's looking at it go away, it will die, and I don't have to worry about Engine surviving past Vehicles destruction, since the auto_ptr will automatically kill it.
However, it makes me feel weird to mix smart pointers and dumb pointers. Do I have any reason for concern?
Your problem is that you now have a circular reference. Engine has a pointer to Vehicle which has a pointer to Engine. This is generally best avoided because things get really complicated when you have to implement copy constructors, assignment operators and destructors.
What are lifecycles of each class and how do you enforce that so that users cannot make mistakes with them?
As I said above, Engine is created by Vehicle, and always owned by Vehicle. As such, it's life cycle is directly associated with it.
As I understand it, this is what they would call a composition type design. A Vehicle is made up of an Engine (and probably other things), whose life-cycle are exactly the same as the Vehicle.
The code is fine, just don't delete your Vehicle pointer in Engine, and understand (as PanGalactic correctly pointed out) that copying Vehicles will give you problems, because auto_ptr only allows a single 'owner', so when you copy the auto_ptr when copying a Vehicle, it will remove the reference to the Engine from the original Vehicile. Example:
1 2 3 4
Vehicle v1;
Vehicle v2(v1);
// At this point, v1.engine_ will not actually point to anything
The above is a bit contrived, but the case is certainly possible if you have a function which takes an Engine as a parameter (by value).
You should override the assignment operator and copy constructor to actually copy the underlying engine into a new auto_ptr, or change auto_ptr to something that reference counts, like boost's shared_ptr.
Well, it's a contrived example, so I'll contrive a case.
Perhaps the Vehicle also has a set of Wheel objects, and the Engine indicates how much force it is providing them. The designer of the system (me) decides that Engine shouldn't duplicate the storage of the Wheels, and that it is not worth the addition of a totally dumb interface class (say DriveTrain) to pass along. Instead, the Engine will use Vehicle as this interface.
Couldn't you just have Vehicle have all of the functions etc that relate it's components? i.e. Vehicle has a "calculateForce()" which would use the engine and tires/whatever and calculate the force.
firedraco, I understand you are trying to help, and I appreciate that. However, as I've said this is just a contrived example, and it isn't my desire to make it water tight. Yes, in this case I could do as you suggest. Let's assume, however, that I have good reason to want Engine to have a pointer to Vehicle. Given this assumption, maybe you can consider the question I asked, rather than trying to find holes in a quickly put together example.
I really do appreciate the help, but I asked a particular question, and mostly people have addressed side issues.
rolie -- Thanks for the response, that is what I thought. I have copy constructors, which I think are doing what I expect, I create a new auto_ptr object, pointing to a copy of the Engine object. I don't call auto_ptr's copy constructor.
I don't see anything wrong with it, as long as you don't let someone delete Engine's Vehicle pointer.
However, I will say that from what you've shown, it seems like your design is kind of flawed. I can't say for sure since I don't have your code exactly but...
Let's assume, however, that I have good reason to want Engine to have a pointer to Vehicle.
Would a const reference work instead? With a reference, it is implicitly understood that the reference holder does not own the referent. And it would prevent someone from doing something like "Engine eng(nullptr);" if they really shouldn't.
One of the big problems with circular references like this is that they make the components difficult to unit test. The pieces are too tightly coupled.
Circular references are a very strong code smell. This is why people are having a tough time answering your question directly. Everyone who has run into them in the past instinctively turns up their nose.
It seems like there are plenty of good reasons why one might want this. What about in a tree, often we wish a node to know about it's parent, or sibling. I understand that circular references ought to be used with care, especially in respect to copy construction, and destruction of the classes.
In the example you cite, the nodes all have pointers to the same type. That makes it easier to reason about the data structure.
Putting the components of your vehicle into a generic container, one whose structure is well-defined, with algorithms to match, is one thing. But that is not what you are doing here.
You are creating a highly specialized data structure. It has not been studied by thousands of computer scientists for decades. There are no generic algorithms that you can use on it. You cannot really equate the two.
It seems like there are plenty of good reasons why one might want this.
There are surprisingly few. One of the impacts of a data structure such as what you are contemplating here is that it makes the design brittle. The object structure is fixed in two directions. To go back to your earlier example, the design is now such that one cannot put a "DriveTrain" class between "Vehicle" and "Engine" without changing both classes.
I'm not saying your design is wrong. I'm just trying to explain why some of us are taking exception to the design.
I don't think there is a right or wrong answer to the design aspect of this question. It really depends what you are using these classes for. That said, a good compromise is to use accessor methods rather than data members directly. If the other members of Engine called Engine::GetVehicle(), and the members of Vehicle called Vehicle::GetEngine(), only those methods would need to be altered to add the DriveTrain middle layer.
Alternately, if everything derived from a common 'VehicleComponant' base class, you could abstract this logic, and GetVehicle() could be a recursive function implemented in the common base class like: