Is this good or bad practice with dynamic memory allocation?

Question

I've seen this used by other people and it looks really clever, but I'm not sure if it's good or bad practice. It works, and I like the way it works, personally, but is doing this actually useful in the scope of a larger program?

What they've done is dynamically allocate some data type inside the actual function argument, and delete it in the function. Here's an example:

#include <iostream>

class Foo {
private:
    int number;
public:
    Foo(int n) : number(n) { }
    int num() { return number; }
    Foo* new_num (int i) { number = i; }
};

void some_func (int thing, Foo* foo);

int main() {
    std::cout << "Enter number: ";
    int n;
    std::cin >> n;
    some_func(n, new Foo(0)); // <-- uses the 'new' operator with a function argument
    return 0;
}

// calculates difference between 'thing' and 'n'
// then puts it inside the Foo object
void some_func (int thing, Foo* foo) {
    std::cout << "Enter another number: ";
    int n;
    std::cin >> n;
    std::cout << "Difference equals " << foo->new_num(thing - n)->num() << std::endl;
    delete foo; // <-- the Foo object is deleted here
}

I knew that it was possible to use operators in function arguments, but I was only aware of doing this with the operators on levels 2, 4 through 15, and 17, as well as the assignment operators, ? :, ++ and --, unary + and -, !, ~, * and &, sizeof and casts. Stuff like this:

foo((x < 3)? 5 : 6, --y * 7);
bar(player->weapon().decr_durability().charge(0.1), &shield_layers);

So, I actually have two questions.

1. Is the new-as-an-argument good practice?

2. Since apparently any operator returning a type works if new works, are using these good practice?

   ::, new [], throw, sizeof..., typeid, noexcept, alignof

Answer 1

No, this is not clever at all. It takes a function that could be simpler and more general and reduces its capabilities for no reason, while at the same time creating an entry point into your program for difficult-to-debug bugs.

It's not clear to me exactly what Foo::new_num is meant to do (right now it doesn't compile), so I won't address your example directly, but consider the following two code samples:

void bad_function(int i, F * f)
{
  f->doSomething(i);
  delete f;
}

// ...

bad_function(0, new F(1, 2, 3));

versus

void good_function(int i, F & f)
{
  f.doSomething(i);
}

// ...

good_function(0, F(1, 2, 3));

In both cases you allocate a new F object as part of the method call and it's destroyed once you're done using it, so you get no advantage by using bad_function instead of good function. However there's a bunch of stuff you can do with good_function that's not so easy to do with bad_function, e.g.

void multi_function(const std::vector<int> & v, F & f)
{
  for(int i : v) { good_function(i, f); }
}

Using the good_function version means you're also prevented by the language itself from doing various things you don't want to do, e.g.

F * f;  // never initialized
bad_function(0, f); // undefined behavior, resulting in a segfault if you're lucky

It's also just better software engineering, because it makes it a lot easier for people to guess what your function does from its signature. If I call a function whose purpose involves reading in a number from the console and doing arithmetic, I absolutely do not expect it to delete the arguments I pass in, and after I spent half an hour figuring out what's causing some obscure crash in some unrelated part of the code I'm going to be furious with whoever wrote that function.

---

By the way, assuming that F::doSomething doesn't alter the value of the current instance of F in any way, it should be declared const:

class F
{
  void doSomething(int i) const;
  // ...
};

and good_function should also take a const argument:

void good_function(int i, const F & f);

This lets anyone looking at the signature confidently deduce that the function won't do anything stupid like mess up the value of f that's passed into the function, because the compiler will prevent it. And that in turn lets them write code more quickly, because it means there's one less thing to worry about.

In fact if I see a function with a signature like bad_function's and there's not an obvious reason for it, then I'd immediately be worried that it's going to do something I don't want and I'd probably read the function before using it.