What is ugly in this C++ function?

[Robertlzw]

Assuming the following function's logic always works:

void f(double* xa, double* xb, double a, double b) {
   assert( xa && xb);
   *xa = *xb + a - b;
   *xb = *xa + 5 * b;
}

If you saw it in a production code, how should you change it for the better?

My answers are (a) playing with pointers are generally more dangerous than using references; In the above function, use references instread. (b) the assert statement on line 2 should throw an exception and add an exception handler for it. Is the following one really better than the original one?

void f(double& xa, double& xb, const double a, const double b) {
   xa = xb + a - b;
   xb = xa + 5 * b;
}

Did I missed any other issues in my answers?

[LB]

Just for extremely minor efficiency, you could also make a and b references. That way copies of them do not have to be made. I usually make all my parameters references, even pointers, unless I want to use the parameter as a local variable.

[Branflakes91093]

Just for extremely minor efficiency, you could also make a and b references.

That could actually make the program less efficient.

[Computergeek01]

With no regard toward efficency (I just deleted a two paragraph rant about how I feel regarding that term) I think this code is ugly because it should be using objects. That would certainly make it look cleaner.

[Disch]

I usually make all my parameters references, even pointers, unless I want to use the parameter as a local variable.

As Branflakes said, this is a bad idea.

References for basic types doesn't prevent any copying. It only adds an indirection, making the code overall slower (potentially)

The rule of thumb is:
- pass basic types (int, double, float, pointers, etc) by value
- pass complex types by const reference (string, vector, etc)


Of course there are exceptions, as there are with most rules.

[Duthomhas]

(b) the assert statement on line 2 should throw an exception and add an exception handler for it. Is the following one really better than the original one?

No. Exceptions are for "release mode". The assert is for debugging purposes, and will only be compiled into the application when compiling in "debug mode".

[ne555]

The code is still obfuscated

void f(double& xa, double& xb, const double a, const double b) {
   xa = xb + a - b;
//   xb = xa + 5 * b;
   xb += a + 4*b; // ¿why 4?
}