Replace boost::optional by std::optional#1947
Conversation
| #if BOOST_VERSION >= 109000 | ||
| # include <optional> | ||
| using std::optional; | ||
| #else | ||
| # include <boost/optional.hpp> | ||
| using boost::optional; | ||
| #endif |
There was a problem hiding this comment.
do we really need this backward support?
There was a problem hiding this comment.
Unless we want to require Boost 1.90+. Prior to that Boost.programOptions didn't support std::optional. I wanted to make it clear when we can switch and remove "old" stuff
There was a problem hiding this comment.
maybe move implementation into cpp file, then we dont need the stdexcept include here
| helpers::OptionalEnum<Job> job = std::nullopt; | ||
| /// Ware produced, if any | ||
| boost_variant2<GoodType, Job, boost::none_t> producedWare = boost::none; | ||
| boost_variant2<GoodType, Job, std::nullopt_t> producedWare = std::nullopt; |
There was a problem hiding this comment.
boost_variant2 is never valueless which makes it easier to work with
| [GetCtrl<ctrlGroup>(ID_grpResolution)->GetCtrl<ctrlComboBox>(ID_cbResolution)->GetSelection().get()]; | ||
| const unsigned windowSizeSelection = | ||
| grpWindowSize.GetCtrl<ctrlComboBox>(ID_cbWindowSize)->GetSelection().get(); | ||
| [*GetCtrl<ctrlGroup>(ID_grpResolution)->GetCtrl<ctrlComboBox>(ID_cbResolution)->GetSelection()]; |
There was a problem hiding this comment.
the * is ambigious, maybe use explicit .value() here, also can we make sure a selection does exist? (clang-tidy with specific settings would cause a unchecked optional access warning here)
There was a problem hiding this comment.
Yeah, I'm missing a get or similar unchecked access method for std::optional
But speed doesn't matter here anyway so .value() it is
| } | ||
| // object wall or impassable terrain increasing my path to target length to a higher value than the direct distance? | ||
| return FindHumanPath(pt, center, CalcDistance(pt, center)) != boost::none; | ||
| return FindHumanPath(pt, center, CalcDistance(pt, center)) != std::nullopt; |
There was a problem hiding this comment.
shouldn't this be a .has_value()?
There was a problem hiding this comment.
Only a matter of style. But sure
2 participants
Unify uses now that we require C++17