Elysian Shadows

Hello everyone, I have a bit of an odd question, so I will do my best to make it clear:

I have a class with an array of objects that I have made private (to follow good programming practice). In order to get properties from an array object, I have a getter that returns an object that i store in a temporary "copy" like so:

//Getter
	Weapon const g_weapon(int i)
	{
		return weapons;
	}

//Calling from another method
Weapon w_temp = deck.g_weapon(i);
w_temp.get_some_property(); //etc.

For memory efficiency, I would like to reference the object instead:

//Getter
	Weapon &g_weapon(int i)
	{	
		Weapon &weapon_ref = weapons;
		return weapon_ref;
	}
//Calling from another method
Weapon &w_ref = deck.g_weapon(i);
w_ref.get_some_property(); //etc.

My questions are:
1 - Is it more memory efficient to use references in the way that I am?
2 - Would I be better off to make the array of Weapon objects public to be begin with? (All of the weapon properties are private with getters and setters)
3 - Also, what is considered best practice?

Thanks,
Josh

1. It is if creating a copy of Weapon is costly (e.g. it has a 1000 element array you have to copy). Actually, if I were to answer your question as-is: yes, passing a reference to an object instead of copying it is always more "memory efficient". Most of the times, though, this memory efficiency is irrelevant. If copying the object only involves duplicating some ints and some short strings, that won't affect performance (unless it's done inside critical sections of the code, but that is another problem).

2. That depends on the rest of the code, but in the context you provided: no, making a member public just to avoid creating a copy on an accessor method is not a good solution.

3. My advice: if you just need to "take a look" at the Weapon (like showing its attributes on the screen), you can return a const reference. If weapons is an array or std::vector, you already have all you need. Both can return const references when accessed. So:

const Weapon & SomeClass::g_weapon(int i) {
    return this->weapons;
}

will give you a const reference through which you can fiddle with all const methods. Just make sure you have the appropriate const accessors on the Weapon:

void f(const SomeClass & some_class) {
    const Weapon & the_weapon = some_class.g_weapon(0);
    // The line below only works if get_some_property has a const version, e.g.
    // int Weapon::get_some_property() const { /* ... */ }
    cout << the_weapon.get_some_property();
}

I hope that answers your question and sorry for the usual verbosity.

Thanks for the reply! Makes sense to me. Based on what you are saying I don't think memory efficiency will be too much of a problem for me (only need a few attributes). I might rewrite a few things just for the practice with references but I probably don't need to.

If you need to return an error condition from your getter, e.g. if you need an error code for if an index doesn't exist, you'll need to use a pointer.

const Weapon* g_weapon(int i)
{
    if (i < 0 || i >= num_weapons)
    {
        return NULL; /* NULL is your error code */
    }
    return &weapons;
}

Unless you want to use exceptions, then you could

#include <exception>
const Weapon& g_weapon(int i)
{
    if (i < 0 || i >= num_weapons)
    {
        throw /* an exception */;
    }
    return weapons;
}

But really you might as well just use a vector so you don't have to deal with arrays, if you're using C++ anyway.

#include <vector>
const Weapon& g_weapon(int i)
{
    return weapons.at(i); /* where weapons is a vector<Weapon> ---- .at() throws an exception if 'i' is out of range */
}

const Weapon* g_weapon(int i)
{
    if (i < 0 || i >= weapons.size()) /* where weapons is a vector<Weapon> */
    {
        return NULL;
    }
    return &weapons; /* doesn't throw an exception */
}

Return a constant reference, unless, as MarauderIIC said, there is an error case. Then return a constant pointer so that you can return NULL.

There is really no point in returning objects like that by value in C/++. You can still keep keep data encapsulated and require modification via a separate set accessor by simply making the return reference/pointer constant. The only exception that I make to this rule is when sizeof(return type) <= sizeof(ptr).

While it is true that the price of returning a trivial object by value is small (and in C++11, even returning nontrivial objects by value isn't so bad with move semantics), that does not mean you should be sloppy about it.

MarauderIIC wrote:But really you might as well just use a vector so you don't have to deal with arrays, if you're using C++ anyway.

It's more of a preference thing, but I kinda cringe when people go for heavier STL-based containers over straight, simple, fast arrays "just cuz." If this isn't going to be resizing, you are gaining nothing, and the template code that instantiating a vector generates isn't what I would call "trivial."