[v1,1/3] libcamera: yaml_parser: Add `YamlObject::find(key)`

22183 diff mbox series

This function is meant to replace `YamlObject::contains()`
as it can be easily used for the same purpose, i.e. determining
if a certain key exists in a dictionary, but it has the advantage
that the result, if any, is immediately available for use
without having to do a second lookup.

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
---
 include/libcamera/internal/yaml_parser.h |  1 +
 src/libcamera/yaml_parser.cpp            | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

Hi Barnabás,

Thank you for the patch.

On Thu, Dec 05, 2024 at 04:34:14PM +0000, Barnabás Pőcze wrote:
> This function is meant to replace `YamlObject::contains()`
> as it can be easily used for the same purpose, i.e. determining
> if a certain key exists in a dictionary, but it has the advantage
> that the result, if any, is immediately available for use
> without having to do a second lookup.
> 
> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
> ---
>  include/libcamera/internal/yaml_parser.h |  1 +
>  src/libcamera/yaml_parser.cpp            | 23 +++++++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
> index 8c791656..796e3e90 100644
> --- a/include/libcamera/internal/yaml_parser.h
> +++ b/include/libcamera/internal/yaml_parser.h
> @@ -209,6 +209,7 @@ public:
>  
>  	bool contains(std::string_view key) const;
>  	const YamlObject &operator[](std::string_view key) const;
> +	const YamlObject *find(std::string_view key) const;
>  
>  private:
>  	LIBCAMERA_DISABLE_COPY_AND_MOVE(YamlObject)
> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
> index db256ec5..da8cb61f 100644
> --- a/src/libcamera/yaml_parser.cpp
> +++ b/src/libcamera/yaml_parser.cpp
> @@ -397,6 +397,29 @@ const YamlObject &YamlObject::operator[](std::string_view key) const
>  	return *iter->second;
>  }
>  
> +/**
> + * \fn YamlObject::find(std::string_view key) const

You can drop this, not required when the documentation block directly
precedes the function.

> + * \brief Retrieve a member by name from the dictionary

Missing \param

> + *
> + * This function retrieves a member of a YamlObject by name. Only YamlObject
> + * instances of Dictionary type associate elements with names, calling this
> + * function on other types of instances or with a nonexistent key results in
> + * \a nullptr being returned.
> + *
> + * \return The YamlObject corresponding to the \a key member
> + */
> +const YamlObject *YamlObject::find(std::string_view key) const
> +{
> +	if (type_ != Type::Dictionary)
> +		return nullptr;
> +
> +	auto iter = dictionary_.find(key);
> +	if (iter == dictionary_.end())
> +		return nullptr;
> +
> +	return iter->second;
> +}

The implementation looks fine. Assuming there's no issue with the way
the function is used in subsequent patches,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
>  #ifndef __DOXYGEN__
>  
>  class YamlParserContext