[libcamera-devel] [PATCH v3 02/11] libcamera: utils: Add string join function

[Laurent Pinchart]

Hi Kieran,

On Tue, Mar 31, 2020 at 01:45:34PM +0100, Kieran Bingham wrote:
> On 20/03/2020 01:48, Laurent Pinchart wrote:
> > Add a utils::join() function to join elements of a container into a
> > string, with a separator and an optional conversion function if the
> > elements are not implicitly convertible to std::string.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/libcamera/include/utils.h | 44 +++++++++++++++++++++++++++++++++++
> >  src/libcamera/utils.cpp       | 16 +++++++++++++
> >  test/utils.cpp                |  7 +++++-
> >  3 files changed, 66 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h
> > index cfa620f24581..242eeded9d50 100644
> > --- a/src/libcamera/include/utils.h
> > +++ b/src/libcamera/include/utils.h
> > @@ -11,6 +11,7 @@
> >  #include <chrono>
> >  #include <memory>
> >  #include <ostream>
> > +#include <sstream>
> >  #include <string>
> >  #include <string.h>
> >  #include <sys/time.h>
> > @@ -109,6 +110,49 @@ inline _hex hex<uint64_t>(uint64_t value, unsigned int width)
> >  
> >  size_t strlcpy(char *dst, const char *src, size_t size);
> >  
> > +#ifndef __DOXYGEN__
> > +template<typename Container, typename UnaryOp>
> > +std::string join(const Container &items, const std::string &sep, UnaryOp op)
> > +{
> > +	std::ostringstream ss;
> > +	bool first = true;
> > +
> > +	for (typename Container::const_iterator it = std::begin(items);
> > +	     it != std::end(items); ++it) {
> > +		if (!first)
> > +			ss << sep;
> > +		else
> > +			first = false;
> > +
> > +		ss << op(*it);
> > +	}
> > +
> > +	return ss.str();
> > +}
> > +
> > +template<typename Container>
> > +std::string join(const Container &items, const std::string &sep)
> > +{
> > +	std::ostringstream ss;
> > +	bool first = true;
> > +
> > +	for (typename Container::const_iterator it = std::begin(items);
> > +	     it != std::end(items); ++it) {
> > +		if (!first)
> > +			ss << sep;
> > +		else
> > +			first = false;
> > +
> > +		ss << *it;
> > +	}
> > +
> > +	return ss.str();
> > +}
> > +#else
> > +template<typename Container, typename UnaryOp>
> > +std::string join(const Container &items, const std::string &sep, UnaryOp op = nullptr);
> > +#endif
> > +
> >  namespace details {
> >  
> >  class StringSplitter
> > diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> > index 58ee7cc19c8c..97f9b632e45b 100644
> > --- a/src/libcamera/utils.cpp
> > +++ b/src/libcamera/utils.cpp
> > @@ -297,6 +297,22 @@ details::StringSplitter::iterator details::StringSplitter::end() const
> >  	return iterator(this, std::string::npos);
> >  }
> >  
> > +/**
> > + * \fn template<typename Container, typename UnaryOp> \
> > + * std::string utils::join(const Container &items, const std::string &sep, UnaryOp op)
> > + * \brief Join elements of a container in a string with a separator
> > + * \param[in] items The container
> > + * \param[in] sep The separator to add between elements
> > + * \param[in] op A function that converts individual elements to strings
> > + *
> > + * This function joins all elements in the \a items container into a string and
> > + * returns it. The \a sep separator is added between elements. If the container
> > + * elements are not implicitly convertible to std::string, the \a op function
> > + * shall be provided to perform conversion of elements to std::string.
> > + *
> > + * \return A string that concatenates all elements in the container
> > + */
> > +
> >  /**
> >   * \fn split(const std::string &str, const std::string &delim)
> >   * \brief Split a string based on a delimiter
> > diff --git a/test/utils.cpp b/test/utils.cpp
> > index 58816f153066..2fca89ef3278 100644
> > --- a/test/utils.cpp
> > +++ b/test/utils.cpp
> > @@ -99,7 +99,7 @@ protected:
> >  			return TestFail;
> >  		}
> >  
> > -		/* utils::split() test. */
> > +		/* utils::join() and utils::split() test. */
> >  		std::vector<std::string> elements = {
> >  			"/bin",
> >  			"/usr/bin",
> > @@ -111,6 +111,11 @@ protected:
> >  		for (const auto &element : elements)
> >  			path += (path.empty() ? "" : ":") + element;
> >  
> > +		if (path != utils::join(elements, ":")) {
> > +			cerr << "utils::join() test failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> 
> This is lacking a test for the optional conversion function ?

Good point. I've added this.

		/* utils::join() with conversion function test. */
		std::vector<Size> sizes = { { 0, 0 }, { 100, 100 } };
		s = utils::join(sizes, "/", [](const Size &size) {
					return size.toString();
				});
 
		if (s != "0x0/100x100") {
			cerr << "utils::join() with conversion test failed" << endl;
			return TestFail;
		}

(I've used Size as I needed something that can't be implicitly converted
to a string, otherwise I would just have used int).

> >  		std::vector<std::string> dirs;
> >  
> >  		for (const auto &dir : utils::split(path, ":"))
> > 

-- 
Regards,

Laurent Pinchart