6

I'm about one week into studying C++ so please bear with me.

So let us say that I have a function that returns a value that I need to use in another function.

Is it better practice for one to assign the returned value to another variable first or is it better to simply call the function as a parameter of the other? Both of which should be valid syntax (I think).

Simplified example:

//method 1
int x = foo(boo(1));

//method 2
int a = boo(1);

int x = foo(a);

I'm starting to need to use functions that involve many parameters, and I can see how method 1 could potentially lead to more confusing code.

//ResolveProcess is a function that resolves dwProcessId using an input

//method 1
HANDLE processHandle = OpenProcess(PROCESS_ALL_ACCESS, FALSE, ResolveProcess(processName));

//method 2
DWORD processID = ResolveProcess(processName);
HANDLE processHandle = OpenProcess(PROCESS_ALL_ACCESS, FALSE, processID);

http://www.learncpp.com doesn't have information on which is better (it usually does), so I figured I'd ask here.

11

In most situations, it's about which version is most readable, which is often in the eye of the beholder. In simple cases, composition is very clear, but past a certain point, verbosity adds clarity. In fact, a lot of people who find boolean parameters less readable will give them a name too so it can be understood without going to the documentation, like:

DWORD processID = ResolveProcess(processName);
BOOL inheritHandle = FALSE;
HANDLE processHandle = OpenProcess(PROCESS_ALL_ACCESS, inheritHandle, processID);

It's also very important to keep in mind with C++ if you have outer(inner()) and inner() returns a new pointer, then outer() must be responsible for freeing it, or you will cause a memory leak. That's not a typical pattern because of the way the dependencies couple.

Most often the function that calls outer(inner()) would be responsible for freeing the memory, and therefore would need to hold onto a reference to the pointer returned by inner(). That's why the composition style is seen a lot less frequently in C++ than in garbage collected languages.

  • 1
    Another style for solving this that I've used (particularly with the Windows API that has a lot of functions with many parameters, e.g. CreateWindowEx which has 12) is to put each parameter on its own line with a comment after it identifying the name of the parameter. – Periata Breatta Sep 16 '16 at 17:13
  • 1
    Not that retuning an owning raw pointer is good C++ anyways, as it goes against RAII. It should be a std::unique_ptr instead. A pity new cannot be redefined to do that too... – Deduplicator Dec 4 '18 at 9:15
4

Which is better depends entirely on the situation.

This:

int x = foo(boo(1));

is called functional composition. It works. It's fine. Unless you need what boo() returned for something other than just passing to foo().

This:

int a = boo(1);

int x = foo(a);

is a missed chance to make the code more readable by giving a a more descriptive name than a. Sure I still have access to a but if I don't need it and this name isn't helping then it's a waste of valuable screen real estate.

A related situation:

a().b();

versus:

C c = a();

c.b();

Again it's hard to see that the two liner is any better especially since it has this useless name.

If you can't be bothered to give it a good name and you only need it for one thing than personally I'm fine with the shorter versions.

If you'd like to read more on this the shorter versions are taking advantage of something called the nameless temporary object.

If you really can't think of a good name but you don't like how dense the one liner is then you can fluff the code like this:

int x = foo(
    boo(1)
);

This style also means that if either throws an exception you'll know where it came from just from the line number.

1

Aside from ownership and readability concerns which are mentioned in the other answers, we should think about debuggability. When a statement becomes large and complex, I ask myself two questions:

  • Would I miss something important when I step over this complete statement in a debugger? If so, split the statement so that each statement only contains one non-trivial operation.

  • Would I want to inspect an intermediate result in a debugger? If so, introduce a variable.

In my experience, designing for debuggability is much more important in C++ than in other languages, since the compiler destroys a lot of useful information that is still accessible than in other languages. In particular, exceptions in Java or Python have stack traces and point to a specific line; exceptions in C++ do not. Once you can reproduce a bug, running the program under a debugger is usually the fastest way of locating the problem.

I have found that composing/nesting function calls very rarely leads to better, simpler code in C++. Independent of language, introducing well-named variables for intermediate results tends to make the code simpler and more self-documenting.

1

If we needed one more reason to separate the calls, adding an extra variable, it's definitely error checking. Your code example uses the Win32 API; and this API doesn't use exceptions and/or RAII. So all return values must be checked.

A function such as ResolveProcess() can fail, and will return an exotic value (e.g. 0, UINT32_MAX, ...). Same for OpenProcess() (you must check that the returned handle is valid) so this should never be passed directly onto the next function. Besides, you'll have to CloseHandle the result if it's not invalid, which is something the construct Frobnicate(OpenProcess(...)) doesn't allow you to do.

Even though your question is not about the Win32 API, in my experience, many times you are tempted to compose calls like this, you end up needing to separate them at the next iteration. Either because you want to check the return values, or you want to log/print/... some information, or because it makes the code clearer, etc.

The good news is: you'll come back at it anyway if you need to. Follow your instinct.

And don't forget to check the error codes.

0

Unfortunately the answer is very much: it depends.

Sometimes, it's clearer and better to assign to a variable, sometimes it's better not to, and there's no single rule that will tell you which to do. However I would suggest three rules of thumb to guide you:

  1. If the function call is a getter then you don't need an intermediate variable.
  2. If the function call has trivially obvious effect then you don't need an intermediate variable.
  3. If the function call has significant side effects then you do need an intermediate variable.

Beyond that it is a matter of taste but consider the differences in what is expressed:

int foodValue = foo(b);
int doodify = doo(d);
float count = bar(foodValue, doodify);

against

float count = bar(foo(b), doo(d));

In the former, you more clearly express what foo(b) and doo(d) are returning (this point is clearer in real code where you have well named variables, of course!) and draw more attention to the calling of the function in these steps. In the latter you are emphasising that the principle step is calling bar and suggesting to the reader that foo and doo are doing less important things. You are encouraging the reader the reader to treat the whole as a unit that they may quickly read over and move to the next thing.

Too much of the former leads to sprawling code that does little with each piece of screen real estate; too much latter leads to dense, impenetrable code which is hard to digest and understand. Only experience can teach you how to balance these competing factors.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.