NovelRT icon indicating copy to clipboard operation
NovelRT copied to clipboard

Consider migrating all `Try` methods to return `std::optional<T>` instead of `bool` with some kind of out `T&`.

Open RubyNova opened this issue 1 year ago • 2 comments

Note: for support questions, please use the #engine-user-help channel in our Discord or create a discussion. This repository's issues are reserved for feature requests and bug reports.

What is the current behaviour? Try methods are currently formatted as bool TryThing(T& outResult). This means users can invoke them and check the bool to see if it succeeded or not, and access the T& in the case of success.

What is the expected behaviour/change? The change would mean the methods would now be formatted as std::optional<T> TryThing(), which would allow users to still perform the same checks as before.

What is the motivation / use case for changing the behavior? The API will be clearer on intent as there is no lingering T& which has to be explained via documentation for those who have not seen that style before. This is a carry-over from C# in NovelRT's original desgin when many of us were more familiar with the dotnet ecosystem, and how they handle this particular case in older APIs. However we are under no such restriction here, and so I believe relying on the standard library's optional type will provide more clarity to end-users.

Describe alternatives you've considered: I have considered simply not doing this and leaving the API as-is, however since functionally it would be the same behaviour I believe it should at least be considered.

I have also considered simply wrapping what we have now in helper methods that return std::optional<T> but I think that would be pointless as all we'd effectively be doing is adding another method in the call stack every time its invoked.

Are there any potential roadblocks or challenges facing this change? The C API mirror for all affected methods would need revisiting - either in implementation, or the implementation + the signature structure. It will also break any internal usages of these methods, meaning all affected code would need to be adjusted so that things compile again.

Are there any downsides to implementing this change? Mostly the boilerplate work outlined in the roadblocks section.

Additional context I've shown a working example in #564 on what the APIs would look like in a real world use case.

RubyNova avatar Feb 06 '23 21:02 RubyNova