views:

185

answers:

2

Hi,

[EDIT 1 - added third pointer syntax (Thanks Alex)]

Which method would you prefer for a DAL and why out of:

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

If unable to find the car first method returns null, second method returns false.

The second method would create a Car object on the heap and populate with data queried from the database. Presumably (my C++ is very rusty) that would mean code along the lines of:

Car& DAL::loadCar(int id)
{
    Car *carPtr = new Car();
    Car &car= *carPtr;
    car.setModel(/* value from database */);
    car.setEngineSize(/* value from database */);
    // etc
    return car;
}

Thanks

+4  A: 

The second is definitely preferable. You are returning a reference to an object that has been new'd. For an end user using the software it is not obvious that the returned object would require deleting. PLUS if the user does something like this

Car myCar = dal.loadCar( id );

The pointer would get lost.

Your second method therefore puts the control of memory on the caller and stops any weird mistakes from occurring.

Edit: Return by reference is sensible but only when the parent, ie DAL, class has control over the lifetime of the reference. ie if the DAL class had a vector of Car objects in it then returning a reference would be a perfectly sensible thing to do.

Edit2: I'd still prefer the second set up. The 3rd is far better than the first but you end up making the caller assume that the object is initialised.

You could also provide

Car DAL::loadCar(int id);

And hope accept the stack copy.

Also don't forget that you can create a kind of null car object so that you return an object that is "valid"ish but returns you no useful information in all the fields (and thus is obviously initialised to rubbish data). This is the Null Object Pattern.

Goz
ng5000
No if the caller wrote what youve just written the memory "could" be free'd. You'd need to call "delete " to do so though .. which looks very odd.
Goz
The stack copy may not even happen: depending on the compiler and the optimizations, the (N)RVO could kick in and make the operation equivalent to #1. In any case, throwing an exception when the car is not found will be necessary.
RaphaelSP
D'oh. The Null Object Pattern removes the need for an exception.
RaphaelSP
+4  A: 

Since you are anyways allocating objects on heap, why not to consider Car * LoadCar() which returns NULL if problem occurs. This way you have no restrictions with reference types (each reference must be initialized) and also have means to signal the error case.

AlexKR
Sounds reasonable, updated question to add a 3rd option.
ng5000
Car there is no null reference, only null pointers.
Massa