[PATCH v1] apps: return std::optional from StreamKeyValueParser::parseRole()

[Barnabás Pőcze]

Hi


2023. február 14., kedd 8:03 keltezéssel, Umang Jain <umang.jain at ideasonboard.com> írta:

> Hi Barnabás,
> 
> Thank you for the patch.
> 
> In the subject line:
>      s/return/Return

ACK


> 
> On 2/13/23 10:13 PM, Barnabás Pőcze wrote:
> > Instead of having bool return type and an out parameter,
> > just use std::optional.
> >
> > Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>
> > ---
> >   src/apps/common/stream_options.cpp | 39 ++++++++++--------------------
> >   src/apps/common/stream_options.h   |  5 ++--
> >   2 files changed, 16 insertions(+), 28 deletions(-)
> >
> > diff --git a/src/apps/common/stream_options.cpp b/src/apps/common/stream_options.cpp
> > index 3a5625f5..d3785999 100644
> > --- a/src/apps/common/stream_options.cpp
> > +++ b/src/apps/common/stream_options.cpp
> > @@ -30,10 +30,8 @@ StreamKeyValueParser::StreamKeyValueParser()
> >   KeyValueParser::Options StreamKeyValueParser::parse(const char *arguments)
> >   {
> >   	KeyValueParser::Options options = KeyValueParser::parse(arguments);
> > -	StreamRole role;
> >
> > -	if (options.valid() && options.isSet("role") &&
> > -	    !parseRole(&role, options)) {
> > +	if (options.valid() && options.isSet("role") && !parseRole(options)) {
> >   		std::cerr << "Unknown stream role "
> >   			  << options["role"].toString() << std::endl;
> >   		options.invalidate();
> > @@ -52,13 +50,8 @@ StreamRoles StreamKeyValueParser::roles(const OptionValue &values)
> >
> >   	StreamRoles roles;
> >   	for (auto const &value : streamParameters) {
> > -		StreamRole role;
> > -
> >   		/* If role is invalid or not set default to viewfinder. */
> > -		if (!parseRole(&role, value.toKeyValues()))
> > -			role = StreamRole::Viewfinder;
> > -
> > -		roles.push_back(role);
> > +		roles.push_back(parseRole(value.toKeyValues()).value_or(StreamRole::Viewfinder));
> >   	}
> 
> The curly braces {...} can be dropped as well

I kept them because of the comment in the previous line.
To me it looks better this way. Should I drop them?


> [...]


Regards,
Barnabás Pőcze