9 ms·

> Pass out-parameters by pointer: Widget*. OP says this like this is an industry rule of sorts but I've pretty much always seen non-const references for this.

• lelanthran 5y ago
  > OP says this like this is an industry rule of sorts but I've pretty much always seen non-const references for this. Using pointers is terrible because now you have to add a check for null inside your function, as its API contract says "I can accept null values", while references make it clear that it does not make sense to call if you don't have a correct object to pass it. The other side of the coin is that it hurts readability if you are passing non-const references. When a function takes a non-const reference, the caller looks like this: foo (bar); // Have to read the header to know if bar is modified. When a function takes a pointer, the caller looks like this: foo (&bar); // Immediately clear that we cannot gloss over this line in code-review. Since I read code much more than I write it, it's easier for me to determine that local non-const variables that are used as arguments to a function are not getting modified. A better (but not best) solution might be to have the language recognise and enforce nullability on arguments. Still not perfect (pointer may be non-null but invalid), but better. One other benefit pointer parameters have over reference parameters is that the API contract is more flexible. For example, I have a function that accepts a network connection, mynet_accept(int listener_fd, char **remote_ip, uint16_t *remote_port); Having the caller specify NULL for `remote_ip` means that the callee just ignores the parameter. Same for `remote_port`. If that was changed to use references, the caller will have to declare variables just to satisfy the parameter list, even if they never intend to use them.
  • jcelerier 5y ago
    > The other side of the coin is that it hurts readability if you are passing non-const references. When a function takes a non-const reference, the caller looks like this: This has honestly never ever been an issue for me. I don't remember the last time where I passed an argument to a C++ function and actually had to wonder if the argument was modified or not. Like, what if they are being modified if that's the called function has to do anyways ? And even if it's const, the implementation can just remove the const which while not a very nice thing to do, is legal in many cases. $ cd $ rg "const_cast" -g '*.cpp' | wc -l 2004 (and that does not even cover the c-style casts) > mynet_accept(int listener_fd, char *remote_ip, uint16_t *remote_port); For this exact case my go-to solution nowadays is struct connection_info { int fd; std::string ip; uint16_t port; }; mynet_accept({ .fd = the_socket, .port = 4556 }); which is I think much more readable than any of the alternatives ( actual example for pretty much this exact case: https://github.com/ossia/libossia/blob/master/examples/Network/udp/OSC_send_receive.cpp https://github.com/ossia/libossia/blob/master/examples/Netwo... )
    • tsimionescu 5y ago
      > Like, what if they are being modified if that's the called function has to do anyways ? The idea is that you have some code like this: x = some_func(); do_something(x); if x == 0 { do_something_else(); } You're trying to track down why do_something_else() wasn't called, let's say. Do you have to look at the definition of do_something()? If your coding standards say "no non-const references", then no. If do_something() was modifying x, it would have looked differently, so you can ignore it.
      • jcelerier 5y ago
        > You're trying to track down why do_something_else() wasn't called, let's say Then I put a breakpoint at that line and hit f5 and don't try to guess what may or may not happen because in the end that's the only way to know what actually happens
        • 5y ago
• Karellen 5y ago
  Or, don't use out-parameters unless you really have to. If a function returns a value, have it return that value, instead of using an out-parameter. Passing in-parameters by const reference rather than by value is a nice win because the semantics of the function remain clear. Input parameters still look like input parameters, you're just preventing unnecessary copies where it's easy to do so. Returning a value as an out-parameter rather than just returning it is a performance hack that prevents a copy, but at the expense of muddying the semantics of the function. As the prophet says: we should forget about small efficiencies, say about 97% of the time. Write code that clearly expresses what it does. Return your return values. If you run a profiler over your code and you notice that some returns are causing a performance problem - sure, at that point, add an overload that takes an out-parameter and use it at the frequenty-executed hot spot.
  • thinkharderdev 5y ago
    I still don't really grok move semantics in C++ but is there a difference between assigning output to an out parameter and returning a value using std::move? Conceptually they seem the same to me but from what little I understand std::move doesn't quite do what I think it does.
    • jcelerier 5y ago
      You must not use std::move when returning a value, just let RVO do its job. The compiler will make it so that the variable you are returning from your function will be constructed in-place at the variable you are assigning the function call to. However, if you are compiling at -O0 without any optimizations this may not happen.
      • kaashif 5y ago
        Since C++17, compilers are now required to elide copies and use RVO: https://en.cppreference.com/w/cpp/language/copy_elision https://en.cppreference.com/w/cpp/language/copy_elision There are probably exceptions and loopholes just like with literally everything in C++, but there's this line: > Return value optimization is mandatory and no longer considered as copy elision; see above.
• GuB-42 5y ago
  I absolutely hate non-const reference parameters. Personally, I never use them except for operator overloading or stl functions. The reason is, as lelanthran said, that when I see func(param), I don't expect param to be modified. The time I lost because of non-const reference parameters adds up to days, maybe weeks. In fact, bugs caused by stealthy references are often way harder to find than null pointer crashes. Null pointer crashes are among the bugs that concern me the least, sure crashes are bad, but the core dump often tells you exactly what's wrong, and they rarely lead to non-DoS exploits. See: offensive programming. Now that I think of it, maybe IDEs should highlight non-const references in function calls. I think it is unfortunate because I like the extra guarantees that references offer over pointers but the lack of explicit (de)referencing is enough for me to avoid.
  • jcelerier 5y ago
    > The time I lost because of non-const reference parameters adds up to days, maybe weeks. Well, we have wildly different experiences, I can't remember a single instance where this was an issue in the projects I've been involved with.
    • GuB-42 5y ago
      And I believe you, here are the possible explanations: - Personal experience: it is not a construct I use, and most of the code I work with don't use it either, so when it happens, it is a surprise. People who come from a pointerless language like Java may expect it. - I work with a lot of messy code: large code bases I barely know, written by many people from different companies, without a style guide. So one function may use references and the next one may use pointers, adding to the confusion. - Of course, documentation is misleading (to the point that I make a conscious effort not to read it), and so are function names. Things like getChild(n) that increments n when pretty much every getChild(n) function everywhere passes n by value. Terrible code, that's why there is a bug in the first place, bugs are usually not in the best written parts. - I actually like jumping in other people terrible code and fix bugs, it is an interesting challenge and I became rather good at it where I work. As a result, I am often given these kinds of job. A few days is not that much compared to my entire experience of debugging terrible code, but enough to be significant. - Of course I spend a lot more time fixing memory corruption (and the problem with stealthy reference is that it can look like memory corruption). But in my experience, references over pointers don't help that much with it. It helps against null pointer dereferencing, but as I said before, these are usually easy bugs to fix.
• quotemstr 5y ago
  > Using pointers is terrible because now you have to add a check for null inside your function, as its API contract says "I can accept null values", while references make it clear that it does not make sense to call if you don't have a correct object to pass it. You need no such NULL check. Specify as part of the function's contract that a pointer be non-NULL: if a caller breaks the contract, behavior is undefined. An unexpected NULL is just like any other violation of the function's contract: a programming error. Spell out the contract in the function's documentation. A contract is more than the signature. There is absolutely no need for a runtime check (certain special cases aside) to enforce function contracts. In some cases, assert()ing that a parameter that is expected to be non-NULL is in fact non-NULL can be useful --- but you don't need such checks for correctness of the program, and use of assertions in general ought to be a matter of taste and discretion. Knee-jerk, automatic checking by random internal functions of a module of pointer parameters for NULL in every single place suggests that the author doesn't fully comprehend the concept of interface contracts generally.
  • nemetroid 5y ago
    It's wildly useful to have interface contracts that can be verified by the compiler. In my experience this benefit of using pointers (vs references) only when the argument is optional way outweighs the drawback of not seeing an '&' at the call site.
    • quotemstr 5y ago
      > In my experience this benefit of using pointers (vs references) only when the argument is optional way outweighs the drawback of not seeing an '&' at the call site. Maybe so. Reasonable people differ on this subject. My point is different: it's just not true that if a parameter is a pointer, then as a general requirement you have to check at runtime whether that pointer is non-NULL. It drives me nuts when I see these checks sprinkled throughout a codebase.
      • nemetroid 5y ago
        > My point is different: it's just not true that if a parameter is a pointer, then as a general requirement you have to check at runtime whether that pointer is non-NULL. It drives me nuts when I see these checks sprinkled throughout a codebase. My point is that if you only ever use pointer arguments to indicate that the argument is optional, then the checks are required.
• flohofwoe 5y ago
  > now you have to add a check for null inside your function While I generally agree that non-const references make more sense for out-parameters (although I actually prefer return values), don't forget that it's possible for a reference to become "null" too. It's quite unlikely to happen, but if it happens it's much worse to debug than a null pointer. Oversimplified example: const int& bla = *((int*)nullptr);
  • mastax 5y ago
    Dereferencing a null pointer is Undefined Behavior.
• huhtenberg 5y ago
  In our corner of said industry it's void foo(const T & in, const T * optional_in, T & out, T * optional_out);
• nyanpasu64 5y ago
  > How would OP make a class, which loads content on construction which must absolutely not being changed after construction then, even within its implementation ? "Being careful" is really not sufficient in my experience, and introducing child classes just to enforce the invariant that "const" provides is actively harmful to readability. Agreed. Also what did you mean by introducing child classes? If you move const fields from your class to a field which is an inner class, either it has an assignment operator (so you can overwrite the entire class by mistake) or not (so the outer class can't be assigned over). If you move all fields from your class to a field, it becomes substantially more tedious to access them. And I suppose you could move immutable fields to be private in a base class supplying a getter, but this feels like a hacky use of inheritance (you lose aggregate initialization, you expose an implementation detail in the header, etc). Personally I want C++'s const fields to allow moving and swapping to replace them, but not regular methods. Is it possible for the compiler to distinguish moves/swaps and regular methods, or provide an escape hatch to allow `operator=() = default` or manually defining moves/swaps? And would the standards committee accept that type of change?
  • mike_hock 5y ago
    I can't see how that would make it better than simply not marking them const. Now you still have to be careful and keep edge cases in mind and deal with "const" fields actually changing. There's one exception, though. I'd support const fields being non-const in the constructor and destructor. But then that should be coupled with the constructor being able to run code before constructing members (in a non-hacky way). The constructor's task is to establish class invariants, so while the constructor is running, it's reasonable to assume all invariants haven't been established, yet, including the immutability of some fields. This isn't nearly as clear-cut for move construction/assignment, which would need to modify a different object's const members, which at the point of invocation is not at any special stage of its lifetime.
• mastax 5y ago
  I really like the way C# did this: void foo<T>(T by_value, ref T by_ref, in T by_immutable_ref, out T out_only); Append `?` to any type to make it optional. (Though that is more of a lint than truly part of the type system). `out` parameters pass no value in and must pass a value out. Isn't that just a return value, semantically? Yes, but we didn't used to have value tuples and destructuring so we had to make do. It can still be nicer for some things: bool TryParse(string input, out int value) if (int.TryParse("123", out int value)) // Use value
  • Jenk 5y ago
    Then async comes along and you can't use `out` in async methods :) Thus (anonymous) tuples were introduce so we can do it Go/Rust-style: async Task<(bool, T)> Foo<T>(T value) { /* do something async like a web request or file read etc. */ var aThing = await GetAThing<T>(value); return (aThing != null, aThing); }