Hi all,

I'm going through an exercise to clean up my code and put some more logic in it, regarding passing references and pointers.

Here's my goal:

** applies to all global functions and member functions of my classes

** if a function parameter is a class which is only input (not to be changed), use a const reference (const &...)

** if a function parameter needs to be altered by the function, use a pointer to the object (*....)

This also gives good readability if a function is changing an object, noticed by "->...".

** when it regards passing 'simple' objects like a vector/vertex, matrix, int, float etc., that is only input (not to be changed), I use const only (no reference). For example:

float DistCoordToCoord(const D3DXVERTEX3 pVector1, const D3DXVECTOR3 pVector2); or

** when it regards passing 'simple' objects which need to be altered, I use a pointer. For example:

void DistCoordToCoord(const D3DXVERTEX3 pVector1, const D3DXVECTOR3 pVector2, float *pResult);

The aim is to get better readability and less fault risks, by applying these style/ correctness.

My question:

- what would you do, looking at this? other approaches/ better approaches?

- am I overseeing things?

First thing I'm gonna do now is create a nice XLS with all classes and functions where this applies.

So any input is appreciated and can be used, since I'll need some time first to get the overview :)

Stops you doing this

void CountDownFrom(int i)
{
    while(i > 0)
    {
        cout << i-- << endl;
    }
}

forcing you to copy the argument. Whether that is a good idea or not, I'll leave you to decide.

EDIT: OP should spend more time making member functions const-correct rather than concentrating solely on function arguments IMHO.

Thanks.

Can you explain a bit more what you mean with making member functions const-correct?

I think it begins with the passed objects to the functions, which should or shouldn't be modified (and prevent copying variables when not needed).

Unless of course, the basics behind are not OK (structure of the classes, what's dependent on what, which member functions should be smaller/ split up etc.).

Not sure if this is the case 100%, but anyway this first step feels like good practice. Next could be to see if I can make it more efficient/ with smaller/ other member functions.

Or do you mean something else?

If you havent already you should also make the methods of objects themselves const:

T object::method(params) const

if they have no side effects on the object (assuming you havent already)

EDIT:

In a few circumstances, instead of doing multiple return values using pointer/reference parameters, i use an std::pair (std::tuple for more than 2 members, havent used it).

For example if i need to query some data on some object at coordinates (x,y), i can return something like std::pair(bool exists, objectData data) so i can see if the object exists.

For larger objects the objectData would go in a reference or pointer parameter though to avoid copying (assuming the compiler doesnt optimize it out?)

Thanks, I understand and read into it.

What I'll also take into account in this 'action', is check which member functions actually change the object or just use vars of the object.

For example most member functions within my 'd3d' class, are const functions (not yet in the code, but I'm gonna change that).

Just on the details, how about a LPDIRECT3DDEVICE9, which is already a pointer. I would say pass it as const reference since it's only used and not changed. Not sure though if the device pointer needs 'mutable' rights to execute functions within in. I hope not, so the code's clean on that side too.

Ps.; in my opinion "const ...." is better readable for me then "..... const", so I'm going for that style, both in the class member functions and the function parameters.

const has to go after the function parameters for a const member function though, so they are not interchangeable.

const int T::foo()

function returns a const int

int T::foo() const

function does not modify the T object which calls foo.

Thanks, I didn't know that.

So to be sure, this would be correct:

class MyClass

{
public:
   int mAge;

   void DoSometingWithNoneMembers(const float p1, float *p2) const;
   void UpdateAge(const int &pAge);
}

void MyClass::DoSometingWithNoneMembers(const float p1, float *p2)

{
   p2 = p1;
}

void MyClass:UpdateAge(const int &pAge)
{
   mAge = *pAge;
}

Where I assume that the 'const' keyword for member functions is only needed in the prototype.

Is this correct, or does is also have to be in the implementation? (sounds logical for readability if it's also there)

You mean *p2 = p1 in example #2. They aren't members of the class so that is OK. EDIT: Why is it a member of the class though? It doesn't use the class at all. If you want it to be a member of the class make it static.

pAge is a reference not a pointer in example #3. So *pAge doesn't make any sense. You mean mAge = pAge. It can't be const though, correct, since it modifies mAge which is a member of the class.

You need the const in the definition too since you can provide a const and a non-const implementation of the same function (the const one will be called if called with a const variable).

I'd also rethink your naming conventions if you use p prefix for all function arguments. I'd use a p prefix for pointers. If you need a naming convention for parameters I'd use "a" for argument.