views:

193

answers:

4

All,

I recently posted this question on DAL design. From that it would seem that passing a reference to an object into a function, with the function then populating that object, would be a good interface for a C++ Data Access Layer, e.g.

  bool DAL::loadCar(int id, Car& car) {}

I'm now wondering if using a reference to a boost::shared_ptr would be better, e.g.

  bool DAL::loadCar(int id, boost::shared_ptr<Car> &car)

Any thoughts? Does one offer advantages over the other?

What would be the implications of applying const correctness to both calls?

Thanks in advance.

A: 

In the first case you simply pass a Car and "fill it" with information. For example, you may create a "default" Car and then fill it. I see one inconvenience in this: it's not very OO to have two classes of Cars: one poor, default, useless, "empty" Car, and one truly filled in Car after it comes from the function. To me, a Car is a Car, so it should be a valid Car (one I can drive from location A to B, for example; one that I can accelerate, brake, start, stop) before and after your function.

I typically work with traditional pointers, not boost (without a problem, by the way) so I really can't comment on the latter alternative.

Daniel Daranas
+2  A: 

It depends on what the function does.

In general, a function taking a pointer indicates that callers might call this function even if they don't have an object to pass to it - they can always pass NULL. If that fits the function's spec, then use a (smart) pointer. Passing reference counting smart pointers by references instead copying them is an optimization (and not a premature one, I should add), because it avoids needlessly increasing and decreasing the reference count, which can, in MT environments, be a noticeable performance hit.

A function taking a non-const reference as an argument expects to be passed a valid object that it might change. Callers cannot (legally) call that function unless they have a valid object and they will not call it unless they are willing to have the function change the object's state. If that better fits the function's spec, use a reference.

sbi
The function must receive a valid object, you should not be able to pass NULL.
ng5000
Then a reference to an object is what you want. Whether that references is is initialized from an object, another reference, a naked point, or a smart pointer, shouldn't bother the function itself.
sbi
+1  A: 

If you must receive a valid object (i.e. you don't want the caller to pass NULL), then by all means, do not use boost::shared_ptr. Your second example passes a reference to a "smart pointer".... ignoring the details, it is a "pointer to pointer to Car". Because it's reference, the shared_ptr object cannot be NULL.... but it doesn't meant that it can't have a NULL value (i.e. point to a "null" object).

I don't understand exactly why you would think that a reference to a smart pointer would be "better" - does the caller function use smart pointer already?

As for the implications of "const"... do you mean something like

bool DAL::loadCar(int id, const Car& car) {}

? If yes, it would be counter-productive, you communicate to the compiler the fact that "car" doesn't change (but presumably you want it to change!).

Or do you mean to make the function "const", something like

class DAL{
   bool loadCar(int id, Car& car) const;
}

?

In the latter case, you comunicate to the compiler/API user that the method "loadCar" does not modify the DAL object. It's a good idea to do so if this is true - not only that it enables some compiler optimizations, but it is generally a good thing to specify in the "contract" (function signature) that the function makes no modifications to DAL, especially if you make this implicit assumption in your code (this way you make sure that this stays true, and that in the future nobody will modify the "loadCar" function in a way that will change the "DAL" object)

Virgil
I just wondered if it were better - I really don't know at the moment which one, if either, is better.
ng5000
Again, with const I wasn't sure. Making the function const is probably the right thing to do, assuming we don't have any counts/caches etc in the DAL.
ng5000
You are hitting the difference between logical const-ness (from the user point of view) and real const-ness (from the dev point of view), I suggest using the logical one > as long as the user does not see any modification, declare the method const. In particular, caches should be declared mutable.
Matthieu M.
+3  A: 

As sbi says, "It depends on what the function does. "

However, I think the most important aspect of the above is not whether NULL is allowed or not, but whether the function stores a pointer to the object for later use. If the function just fills in some data then I would use reference for the following reasons:

  • the function can still be used by clients who do not use shared_ptr, used for stack objects, etc.
  • using the function with shared_ptr is still trivial - shared_ptr has dereferencing operator that returns a reference
  • passing NULL is not possible
  • less typing
  • I don't like using "stuff" when I don't have to

If the function needs to store pointer for later use or you anticipate the function might change in such a way that will require storing a pointer, then use shared_ptr.

sbk