Closed OlafvdSpek closed 1 year ago
and 2. have undefined behavior if it's a nullopt
The example code doesn't have any UB because the values exist, so using operator*
is safe. The reader understanding this usage of std::optional
is an assumption made for the sake of the example.
In normal, non-demo code you should be checking the presence of a value in the returned std::optional
, of course. That example doesn't do that because it's more about showing the different ways you can query data, not boring error-handling code we've all written 1000 times and is somewhat out-of-scope. The specific documentation example for value() has that covered already.
There of course will be people who don't understand that std::optional
may be nullopt
, but that's not really a problem my library needs to address - that is solved simply by reading cppreference.com. std::optional
is a vocabulary type and it's use should be encouraged, not avoided.
(as an aside: the library may be used with exceptions disabled completely, so changing value()
to rely on exceptions is a non-starter for people relying on that)
Oh, also: toml::table
and toml::array
both have an at()
method that will throw that exception in the same way a std::vector::at()
does, so you can actually already get this behaviour if you first get the source node as a table/array:
These are intended for more advanced use, whereas value()
'just works' regardless of what the concrete node actually is, so you can pick your poison.
is an assumption made for the sake of the example.
It's not like using .value()
makes the example (significantly) more complex.
so changing value() to rely on exceptions is a non-starter for people relying on that)
I'm not requesting existing semantics to be changed.
I'm not requesting existing semantics to be changed.
Yeah, I initially mis-read your request.
In any case my points about std::optional
being a vocabulary type are valid; I could implement something like value_or_throw()
, or people could just use std::optional::value()
for checked-access semantics, if that's what they need. The standard library already has you covered here for the general case, and for more advanced usage requiring better error mesages this is already implemented via at()
.
I suppose I could update the example to use std::optional::value()
, though I maintain it's out of scope and irrelevant which method is used as the example isn't about teaching people how to use the standard library, but merely demonstrating the different ways to query node values in toml++.
If you genuinely believe the example should be changed to emphasize the specifics of using std::optional
here then I'll happily accept a PR, but otherwise I don't think I'll bother.
In any case my points about
std::optional
being a vocabulary type are valid; I could implement something likevalue_or_throw()
, or people could just usestd::optional::value()
for checked-access semantics, if that's what they need. The standard library already has you covered here for the general case, and for more advanced usage requiring better error mesages this is already implemented viaat()
.
at()
indeed perfectly covers "A key access function that'd throw on key not found with an error message containing the name of the key would be helpful."
What about "A value access function that'd throw on wrong data type might be helpful too." though?
bad optional access
doesn't provide enough useful info to the user.
I'll think about it. Having both complicates the implementation quite a bit so I'd rather not.
In the interim if you want a message that says something like value error: tried to get type 'bool' as type 'string'
all the tools to do this are in the library now and could be implemented easily in a helper function in your own code.
all the tools to do this are in the library now and could be implemented easily in a helper function in your own code.
True. But if multiple library users have to write the same helper function, it'd make sense to just include it in the library.
it'd make sense to just include it in the library.
What 'makes sense' is up to me. I said I'd think about it, but to reiterate: this implies a very annoying complication of the implementation, so currently I am at a firm "maybe", especially since you're the first user in a number of years to ask for this. The solution I suggested above was an interim solution. Let's leave it at that.
Is your feature request related to a problem? Please describe.
.value()
might be safer..value()
would throwbad optional access
, which doesn't tell a lot about the cause or location of the issue.https://marzer.github.io/tomlplusplus/#mainpage-example-manipulations
Describe the solution you'd like A key access function that'd throw on key not found with an error message containing the name of the key would be helpful. A value access function that'd throw on wrong data type might be helpful too.