8 ms·

Agreed. This code just looks weird to me. Throwing from a constructor? Holding the result of new() in a temporary? This is just asking for trouble. Another fav

by blake1 5y ago

Agreed. This code just looks weird to me. Throwing from a constructor? Holding the result of new() in a temporary? This is just asking for trouble. Another favorite “criticism” I have of C++ goes along the lines of “but someone could overload operator+ to be division!” I have only seen that as a contrived critic’s example, or a joke.

Koshkin 5y ago
> Throwing from a constructor? Actually, that’s what you are supposed to do, as this is the only way for a constructor to let the caller know that something went wrong.
- remram 5y ago
  There is always the option to expose the "fallible construction" as a factory function instead of a constructor. Then you can return errors as needed.
  - jnwatson 5y ago
    And then every object has one more state.
    - Thorrez 5y ago
      You mean the factory function has to return some invalid state of the class? Not so. The factory could return std::optional<Foo> or std::unique_ptr<Foo> or std::variant<Foo, Error> or absl::StatusOr<Foo> .
      - jcelerier 5y ago
        Your program will still have as many possible memory states as if you had a bool m_isValid; in your type - as now you don't store a Foo but an optional<Foo>. It's an improvement in terms of code reuse but not in semantic simplification basically.
        pwdisswordfish8 5y ago
        You realise you can just unwrap an optional after checking it’s not nullopt, right?
        jcelerier 5y ago
        Not if it's part of the state of your app ? E.g. a struct member. It also replaces an automated and enforced action by the compiler (exception being thrown) by a need for a manual check which feels very 1975
        Thorrez 5y ago
        >Not if it's part of the state of your app ? E.g. a struct member. I'm not sure what you mean. Here's an example with the output from std::optional being unwrapped and put in a struct member: https://godbolt.org/z/Ms4Kar18j https://godbolt.org/z/Ms4Kar18j Also there's opt.value_or() which can use a default value in the case opt is nullopt. >It also replaces an automated and enforced action by the compiler (exception being thrown) by a need for a manual check which feels very 1975 I think remram's goal was to avoid exceptions. So this is a criticism of remram's goal rather than the method to achieve that goal. Also, std::optional can integrate with exception-producing code because opt.value() throws an exception if opt is nullopt.
        jcelerier 5y ago
        > I'm not sure what you mean. Here's an example with the output from std::optional being unwrapped and put in a struct member: https://godbolt.org/z/Ms4Kar18j https://godbolt.org/z/Ms4Kar18j And now you have to do this for every type that have some amount of preconditions. I don't understand in which universe this is sane - more code == more bugs. e.g. here's what one would write with exceptions: https://godbolt.org/z/evMsh6383 https://godbolt.org/z/evMsh6383 (and, to be honest, if one is writing e.g. a command-line tool and not a reactive gui app, most likely https://godbolt.org/z/eW8YM5Gz4 https://godbolt.org/z/eW8YM5Gz4 as your OS will catch the thing anyways and then it's just a `coredumpctl gdb` / "open in debugger" away) ; you get the same guarantee of never having an invalid Foo but at much less mental cost. It also prevents you of having aggregates as all the "parent" owner class will need their own wrapping in optional + private ctor, in case a sub-sub-sub-sub field would fail. e.g. given struct DomainObject { Foo mainFoo; Foo secondaryFoo; int whatever; }; struct GameState { std::vector<DomainObject> objects; }; now you can't just do DomainObject{initForMainFoo, initForSecondaryFoo, 123}; or DomainObject{.mainFoo = "", .secondaryFoo = "", .whatever = 123}; anymore, even less GameState{{DomainObject{...}, DomainObject{...}}; which is how modern C++ is meant to be used
    - rualca 5y ago
      > And then every object has one more state. No, not really. Either the allocation followed the happy path, or one of the expected failure modes kicked in. And the supposed footgun example boiled down to failing to handle one of the failure modes. I mean, think about it: std::make_shared is a factory method, isn't it?
  - Koshkin 5y ago
    Not always, e.g. not if your class includes a member that is a reference (and, say, aggregate initialization is not allowed for one reason or another).
    - Thorrez 5y ago
      It seems to work for me: https://godbolt.org/z/68s8qsP6K https://godbolt.org/z/68s8qsP6K
  - TeMPOraL 5y ago
    That's an option, but now you're using a factory method instead of a constructor. It's a valid choice, particularly if you want to go for exception-free style - but GP's main point was that throwing from a constructor is not only not a footgun, it's actually the correct practice if you're writing RAII style.
  - layer8 5y ago
    That doesn’t compose in a class hierarchy, though.
    - remram 5y ago
      That's a good point... You might be able to work around that using a template function factory e.g. `make_foobar<ChildrenFooBar>()`. I'm not sure what's a good pattern here, I don't write much C++.
    - gpderetta 5y ago
      As long as the every element of the hierarchy is noexcept-movable it still composes. It is just very cumbersome.
- fpoling 5y ago
  You can also use out parameters to tell about errors.
  - Koshkin 5y ago
    This won't work with third-party callers.
  - johannes1234321 5y ago
    When doing that the better approach is to use a factory function wrapping this. However mind that if you don't throw from C++'s perspective the object is fully created, so that the destructor will run. If you throw an exception from the contractor, the object won't be fully created.
    - fpoling 5y ago
      In practice I have not found that running destructor is problematic. In many cases the destructor is the default one that just destructs the fields. When the destructor should be non-default the constructor can leave the object in the move out state either explicitly or via hacks like using T tmp(std::move((*this)) on the error return in the constructor.
cglodt 5y ago
Throwing from a constructor is a fundamental way to make it impossible to construct invalid instances of a class. It's great, especially for immutable classes. Throwing from constructor + immutability = no invalid states ever.
LAC-Tech 5y ago
> Another favorite “criticism” I have of C++ goes along the lines of “but someone could overload operator+ to be division!” I have only seen that as a contrived critic’s example, or a joke. Yeah there's definitely criticisms you can throw at C++, but this always struck me as stupid. You could also write an `add` method that divides in Java. I've never liked the idea of 'operators' in the first place, tbh. Scheme opened my eyes in that they're all just procedures. And then with Smalltalk and Scala I saw that they can all just be methods as well.

by blake1 5y ago

Koshkin 5y ago

> Throwing from a constructor? Actually, that’s what you are supposed to do, as this is the only way for a constructor to let the caller know that something went wrong.

remram 5y ago
There is always the option to expose the "fallible construction" as a factory function instead of a constructor. Then you can return errors as needed.
- jnwatson 5y ago
  And then every object has one more state.
  - Thorrez 5y ago
    You mean the factory function has to return some invalid state of the class? Not so. The factory could return std::optional<Foo> or std::unique_ptr<Foo> or std::variant<Foo, Error> or absl::StatusOr<Foo> .
    - jcelerier 5y ago
      Your program will still have as many possible memory states as if you had a bool m_isValid; in your type - as now you don't store a Foo but an optional<Foo>. It's an improvement in terms of code reuse but not in semantic simplification basically.
      - pwdisswordfish8 5y ago
        You realise you can just unwrap an optional after checking it’s not nullopt, right?
        jcelerier 5y ago
        Not if it's part of the state of your app ? E.g. a struct member. It also replaces an automated and enforced action by the compiler (exception being thrown) by a need for a manual check which feels very 1975
        Thorrez 5y ago
        >Not if it's part of the state of your app ? E.g. a struct member. I'm not sure what you mean. Here's an example with the output from std::optional being unwrapped and put in a struct member: https://godbolt.org/z/Ms4Kar18j https://godbolt.org/z/Ms4Kar18j Also there's opt.value_or() which can use a default value in the case opt is nullopt. >It also replaces an automated and enforced action by the compiler (exception being thrown) by a need for a manual check which feels very 1975 I think remram's goal was to avoid exceptions. So this is a criticism of remram's goal rather than the method to achieve that goal. Also, std::optional can integrate with exception-producing code because opt.value() throws an exception if opt is nullopt.
        jcelerier 5y ago
        > I'm not sure what you mean. Here's an example with the output from std::optional being unwrapped and put in a struct member: https://godbolt.org/z/Ms4Kar18j https://godbolt.org/z/Ms4Kar18j And now you have to do this for every type that have some amount of preconditions. I don't understand in which universe this is sane - more code == more bugs. e.g. here's what one would write with exceptions: https://godbolt.org/z/evMsh6383 https://godbolt.org/z/evMsh6383 (and, to be honest, if one is writing e.g. a command-line tool and not a reactive gui app, most likely https://godbolt.org/z/eW8YM5Gz4 https://godbolt.org/z/eW8YM5Gz4 as your OS will catch the thing anyways and then it's just a `coredumpctl gdb` / "open in debugger" away) ; you get the same guarantee of never having an invalid Foo but at much less mental cost. It also prevents you of having aggregates as all the "parent" owner class will need their own wrapping in optional + private ctor, in case a sub-sub-sub-sub field would fail. e.g. given struct DomainObject { Foo mainFoo; Foo secondaryFoo; int whatever; }; struct GameState { std::vector<DomainObject> objects; }; now you can't just do DomainObject{initForMainFoo, initForSecondaryFoo, 123}; or DomainObject{.mainFoo = "", .secondaryFoo = "", .whatever = 123}; anymore, even less GameState{{DomainObject{...}, DomainObject{...}}; which is how modern C++ is meant to be used
  - rualca 5y ago
    > And then every object has one more state. No, not really. Either the allocation followed the happy path, or one of the expected failure modes kicked in. And the supposed footgun example boiled down to failing to handle one of the failure modes. I mean, think about it: std::make_shared is a factory method, isn't it?
- Koshkin 5y ago
  Not always, e.g. not if your class includes a member that is a reference (and, say, aggregate initialization is not allowed for one reason or another).
  - Thorrez 5y ago
    It seems to work for me: https://godbolt.org/z/68s8qsP6K https://godbolt.org/z/68s8qsP6K
- TeMPOraL 5y ago
  That's an option, but now you're using a factory method instead of a constructor. It's a valid choice, particularly if you want to go for exception-free style - but GP's main point was that throwing from a constructor is not only not a footgun, it's actually the correct practice if you're writing RAII style.
- layer8 5y ago
  That doesn’t compose in a class hierarchy, though.
  - remram 5y ago
    That's a good point... You might be able to work around that using a template function factory e.g. `make_foobar<ChildrenFooBar>()`. I'm not sure what's a good pattern here, I don't write much C++.
  - gpderetta 5y ago
    As long as the every element of the hierarchy is noexcept-movable it still composes. It is just very cumbersome.
fpoling 5y ago
You can also use out parameters to tell about errors.
- Koshkin 5y ago
  This won't work with third-party callers.
- johannes1234321 5y ago
  When doing that the better approach is to use a factory function wrapping this. However mind that if you don't throw from C++'s perspective the object is fully created, so that the destructor will run. If you throw an exception from the contractor, the object won't be fully created.
  - fpoling 5y ago
    In practice I have not found that running destructor is problematic. In many cases the destructor is the default one that just destructs the fields. When the destructor should be non-default the constructor can leave the object in the move out state either explicitly or via hacks like using T tmp(std::move((*this)) on the error return in the constructor.

cglodt 5y ago

Throwing from a constructor is a fundamental way to make it impossible to construct invalid instances of a class. It's great, especially for immutable classes. Throwing from constructor + immutability = no invalid states ever.

LAC-Tech 5y ago

> Another favorite “criticism” I have of C++ goes along the lines of “but someone could overload operator+ to be division!” I have only seen that as a contrived critic’s example, or a joke. Yeah there's definitely criticisms you can throw at C++, but this always struck me as stupid. You could also write an `add` method that divides in Java. I've never liked the idea of 'operators' in the first place, tbh. Scheme opened my eyes in that they're all just procedures. And then with Smalltalk and Scala I saw that they can all just be methods as well.