[libcamera-devel] [PATCH v2 02/10] libcamera: utils: Add string join function

[Laurent Pinchart]

Hi Jacopo,

On Tue, Mar 17, 2020 at 01:06:59PM +0100, Jacopo Mondi wrote:
> On Mon, Mar 16, 2020 at 11:43:02PM +0200, 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>
> > ---
> >  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 940597760ee2..74ceed760cb3 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);
> 
> What does thi give us ?
> 
> op is defaulted to nullptr, but it requires the template argument to
> be specified anyway...

This is for doxygen only, in order to avoid documenting two functions
that are otherwise identical. I haven't found a way to merge both
implementations into a single template function that would make the
compiler happy.

> Apart from this
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> > +#endif
> > +
> >  namespace details {
> >
> >  class StringSplitter
> > diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> > index f566e88cec5b..1d0e583756cf 100644
> > --- a/src/libcamera/utils.cpp
> > +++ b/src/libcamera/utils.cpp
> > @@ -291,6 +291,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;
> > +		}
> > +
> >  		std::vector<std::string> dirs;
> >
> >  		for (const auto &dir : utils::split(path, ":"))

-- 
Regards,

Laurent Pinchart