Skip to content

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

@RubyNova

Description

@RubyNova

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.

Metadata

Metadata

Labels

dev team approvedA proposal that the development team have approved for implementation.engine apiTickets pertaining to NovelRT's end-user API.engine coreTickets pertaining to the engine core codebase.featureA feature ticket for the NovelRT.proposalA proposal up for debate.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions