Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add helper class to store optional references. #414

Merged
merged 1 commit into from
Oct 24, 2023

Conversation

tquatmann
Copy link
Member

Adds storm::OptionalRef, a helper class that optionally holds a reference to an object.
This mimics the interface of std::optional, except that an OptionalRef never takes ownership of an object.

A possible use case is the implementation of optional function arguments, where std::optional would trigger a deep copy of the object. For example, foo(storm::OptionalRef<T const> bar = storm::NullRef) instead of foo(T const& bar).

Why?

This seems to be the best option for providing optional arguments:

  • We use void foo(std::optional<A const> const& a) {...} a lot, but it's imo very implicit that e.g. in foo(bar); a copy of bar is created when it is of type A or A const& .
  • Sometimes, we use a raw pointer like void foo(A const* a) {...} which avoids a copy but it's not very explicit about the fact that a is optional and it makes ownership unclear.
  • std::optional<A const&> is not allowed by the CPP standard.
  • We could use void foo(std::optional<std::reference_wrapper<A>> const& a) { ... } but it becomes a bit annoying since in the body of foo we'd have to deal with both, the optional and the reference_wrapper, e.g. A const& a_ref = a.value().get()
  • Function overloading (i.e. have both a foo() and a foo(A const& a) is not always feasible (code duplications, dealing with multiple optional arguments, ...)

@volkm
Copy link
Contributor

volkm commented Oct 24, 2023

LGTM

@tquatmann tquatmann merged commit 96e6072 into moves-rwth:master Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants