[libcamera-devel,5/7] libcamera: yaml_parser: Remove memberNames() function
diff mbox series

Message ID 20220616142403.20723-6-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: yaml_parser: Add iterator API
Related show

Commit Message

Laurent Pinchart June 16, 2022, 2:24 p.m. UTC 
Now that YamlObject supports iteration, the memberNames() function isn't
useful anymore as it can be implemented using utils::map_keys() if
really needed. Drop it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/internal/yaml_parser.h |  1 -
 src/libcamera/yaml_parser.cpp            | 22 ----------------------
 test/yaml-parser.cpp                     | 10 ----------
 3 files changed, 33 deletions(-)

Comments

Jacopo Mondi June 17, 2022, 1:50 p.m. UTC | #1 
Hi Laurent

On Thu, Jun 16, 2022 at 05:24:01PM +0300, Laurent Pinchart via libcamera-devel wrote:
> Now that YamlObject supports iteration, the memberNames() function isn't
> useful anymore as it can be implemented using utils::map_keys() if
> really needed. Drop it.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> ---
>  include/libcamera/internal/yaml_parser.h |  1 -
>  src/libcamera/yaml_parser.cpp            | 22 ----------------------
>  test/yaml-parser.cpp                     | 10 ----------
>  3 files changed, 33 deletions(-)
>
> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
> index 8edd32354184..811be1affcc0 100644
> --- a/include/libcamera/internal/yaml_parser.h
> +++ b/include/libcamera/internal/yaml_parser.h
> @@ -172,7 +172,6 @@ public:
>
>  	bool contains(const std::string &key) const;
>  	const YamlObject &operator[](const std::string &key) const;
> -	std::vector<std::string> memberNames() const;
>
>  private:
>  	LIBCAMERA_DISABLE_COPY_AND_MOVE(YamlObject)
> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
> index 4df7e5a33d47..9b6e70cbfcf3 100644
> --- a/src/libcamera/yaml_parser.cpp
> +++ b/src/libcamera/yaml_parser.cpp
> @@ -335,28 +335,6 @@ bool YamlObject::contains(const std::string &key) const
>  	return true;
>  }
>
> -/**
> - * \fn YamlObject::memberNames()
> - * \brief Retrieve all member names of the dictionary
> - *
> - * This function retrieve member names of a YamlObject. Only YamlObject
> - * instances of Dictionary type associate elements with names, calling this
> - * function on other types of instances is invalid and results in undefined
> - * behaviour.
> - *
> - * \todo Replace this function with an iterator-based API
> - *
> - * \return A vector of string as the member names
> - */
> -std::vector<std::string> YamlObject::memberNames() const
> -{
> -	std::vector<std::string> memberNames;
> -	for (auto &[key, _] : dictionary_)
> -		memberNames.push_back(key);
> -
> -	return memberNames;
> -}
> -
>  /**
>   * \fn YamlObject::operator[](const std::string &key) const
>   * \brief Retrieve a member by name from the dictionary
> diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp
> index ab6e9a01c0fe..38f848232fa6 100644
> --- a/test/yaml-parser.cpp
> +++ b/test/yaml-parser.cpp
> @@ -500,16 +500,6 @@ protected:
>  		/* Make sure utils::map_keys() works on the adapter. */
>  		(void)utils::map_keys(dictObj.asDict());
>
> -		auto memeberNames = dictObj.memberNames();
> -		sort(memeberNames.begin(), memeberNames.end());
> -
> -		if (memeberNames[0] != "a" ||
> -		    memeberNames[1] != "b" ||
> -		    memeberNames[2] != "c") {
> -			cerr << "Dictionary object fail to parse member names" << std::endl;
> -			return TestFail;
> -		}
> -
>  		/* Test leveled objects */
>  		auto &level1Obj = (*root)["level1"];
>
> --
> Regards,
>
> Laurent Pinchart
>
Hanlin Chen June 20, 2022, 12:03 p.m. UTC | #2 
Hi Laurent,

On Fri, Jun 17, 2022 at 9:50 PM Jacopo Mondi via libcamera-devel
<libcamera-devel@lists.libcamera.org> wrote:
>
> Hi Laurent
>
> On Thu, Jun 16, 2022 at 05:24:01PM +0300, Laurent Pinchart via libcamera-devel wrote:
> > Now that YamlObject supports iteration, the memberNames() function isn't
> > useful anymore as it can be implemented using utils::map_keys() if
> > really needed. Drop it.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Han-Lin Chen <hanlinchen@chromium.org>

Thanks.

>
> Thanks
>   j
>
> > ---
> >  include/libcamera/internal/yaml_parser.h |  1 -
> >  src/libcamera/yaml_parser.cpp            | 22 ----------------------
> >  test/yaml-parser.cpp                     | 10 ----------
> >  3 files changed, 33 deletions(-)
> >
> > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
> > index 8edd32354184..811be1affcc0 100644
> > --- a/include/libcamera/internal/yaml_parser.h
> > +++ b/include/libcamera/internal/yaml_parser.h
> > @@ -172,7 +172,6 @@ public:
> >
> >       bool contains(const std::string &key) const;
> >       const YamlObject &operator[](const std::string &key) const;
> > -     std::vector<std::string> memberNames() const;
> >
> >  private:
> >       LIBCAMERA_DISABLE_COPY_AND_MOVE(YamlObject)
> > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
> > index 4df7e5a33d47..9b6e70cbfcf3 100644
> > --- a/src/libcamera/yaml_parser.cpp
> > +++ b/src/libcamera/yaml_parser.cpp
> > @@ -335,28 +335,6 @@ bool YamlObject::contains(const std::string &key) const
> >       return true;
> >  }
> >
> > -/**
> > - * \fn YamlObject::memberNames()
> > - * \brief Retrieve all member names of the dictionary
> > - *
> > - * This function retrieve member names of a YamlObject. Only YamlObject
> > - * instances of Dictionary type associate elements with names, calling this
> > - * function on other types of instances is invalid and results in undefined
> > - * behaviour.
> > - *
> > - * \todo Replace this function with an iterator-based API
> > - *
> > - * \return A vector of string as the member names
> > - */
> > -std::vector<std::string> YamlObject::memberNames() const
> > -{
> > -     std::vector<std::string> memberNames;
> > -     for (auto &[key, _] : dictionary_)
> > -             memberNames.push_back(key);
> > -
> > -     return memberNames;
> > -}
> > -
> >  /**
> >   * \fn YamlObject::operator[](const std::string &key) const
> >   * \brief Retrieve a member by name from the dictionary
> > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp
> > index ab6e9a01c0fe..38f848232fa6 100644
> > --- a/test/yaml-parser.cpp
> > +++ b/test/yaml-parser.cpp
> > @@ -500,16 +500,6 @@ protected:
> >               /* Make sure utils::map_keys() works on the adapter. */
> >               (void)utils::map_keys(dictObj.asDict());
> >
> > -             auto memeberNames = dictObj.memberNames();
> > -             sort(memeberNames.begin(), memeberNames.end());
> > -
> > -             if (memeberNames[0] != "a" ||
> > -                 memeberNames[1] != "b" ||
> > -                 memeberNames[2] != "c") {
> > -                     cerr << "Dictionary object fail to parse member names" << std::endl;
> > -                     return TestFail;
> > -             }
> > -
> >               /* Test leveled objects */
> >               auto &level1Obj = (*root)["level1"];
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> >

Patch
diff mbox series 

diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
index 8edd32354184..811be1affcc0 100644
--- a/include/libcamera/internal/yaml_parser.h
+++ b/include/libcamera/internal/yaml_parser.h
@@ -172,7 +172,6 @@  public:
 
 	bool contains(const std::string &key) const;
 	const YamlObject &operator[](const std::string &key) const;
-	std::vector<std::string> memberNames() const;
 
 private:
 	LIBCAMERA_DISABLE_COPY_AND_MOVE(YamlObject)
diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
index 4df7e5a33d47..9b6e70cbfcf3 100644
--- a/src/libcamera/yaml_parser.cpp
+++ b/src/libcamera/yaml_parser.cpp
@@ -335,28 +335,6 @@  bool YamlObject::contains(const std::string &key) const
 	return true;
 }
 
-/**
- * \fn YamlObject::memberNames()
- * \brief Retrieve all member names of the dictionary
- *
- * This function retrieve member names of a YamlObject. Only YamlObject
- * instances of Dictionary type associate elements with names, calling this
- * function on other types of instances is invalid and results in undefined
- * behaviour.
- *
- * \todo Replace this function with an iterator-based API
- *
- * \return A vector of string as the member names
- */
-std::vector<std::string> YamlObject::memberNames() const
-{
-	std::vector<std::string> memberNames;
-	for (auto &[key, _] : dictionary_)
-		memberNames.push_back(key);
-
-	return memberNames;
-}
-
 /**
  * \fn YamlObject::operator[](const std::string &key) const
  * \brief Retrieve a member by name from the dictionary
diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp
index ab6e9a01c0fe..38f848232fa6 100644
--- a/test/yaml-parser.cpp
+++ b/test/yaml-parser.cpp
@@ -500,16 +500,6 @@  protected:
 		/* Make sure utils::map_keys() works on the adapter. */
 		(void)utils::map_keys(dictObj.asDict());
 
-		auto memeberNames = dictObj.memberNames();
-		sort(memeberNames.begin(), memeberNames.end());
-
-		if (memeberNames[0] != "a" ||
-		    memeberNames[1] != "b" ||
-		    memeberNames[2] != "c") {
-			cerr << "Dictionary object fail to parse member names" << std::endl;
-			return TestFail;
-		}
-
 		/* Test leveled objects */
 		auto &level1Obj = (*root)["level1"];