[libcamera-devel] [PATCH v7 06/11] lc-compliance: Move role to string conversion to its own function

[Laurent Pinchart]

Hi Nícolas,

Thank you for the patch.

On Thu, Jul 22, 2021 at 08:28:46PM -0300, Nícolas F. R. A. Prado wrote:
> The functions that generate the test name based on the parameters need
> to convert a StreamRole to a string. Move this to a separate function to
> avoid redundancy.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>
> ---
> 
> No changes in v7
> 
> No changes in v6
> 
> Added in v5
> 
>  src/lc-compliance/capture_test.cpp | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/src/lc-compliance/capture_test.cpp b/src/lc-compliance/capture_test.cpp
> index 949407d3191e..b4807486ee07 100644
> --- a/src/lc-compliance/capture_test.cpp
> +++ b/src/lc-compliance/capture_test.cpp
> @@ -18,6 +18,16 @@ using namespace libcamera;
>  const std::vector<int> NUMREQUESTS = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };
>  const std::vector<StreamRole> ROLES = { Raw, StillCapture, VideoRecording, Viewfinder };
>  
> +static const std::string &roleToString(const StreamRole &role)
> +{
> +	static std::map<StreamRole, std::string> rolesMap = { { Raw, "Raw" },
> +							      { StillCapture, "StillCapture" },
> +							      { VideoRecording, "VideoRecording" },
> +							      { Viewfinder, "Viewfinder" } };

This should be const. I'd write it as

	static const std::map<StreamRole, std::string> rolesMap = {
		{ Raw, "Raw" },
		{ StillCapture, "StillCapture" },
		{ VideoRecording, "VideoRecording" },
		{ Viewfinder, "Viewfinder" },
	};

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

> +
> +	return rolesMap[role];
> +}
> +
>  class CameraHolder
>  {
>  protected:
> @@ -71,15 +81,8 @@ void SingleStream::TearDown()
>  
>  std::string SingleStream::nameParameters(const testing::TestParamInfo<SingleStream::ParamType> &info)
>  {
> -	std::map<StreamRole, std::string> rolesMap = { { Raw, "Raw" },
> -						       { StillCapture, "StillCapture" },
> -						       { VideoRecording, "VideoRecording" },
> -						       { Viewfinder, "Viewfinder" } };
> -
> -	std::string roleName = rolesMap[std::get<0>(info.param)];
> -	std::string numRequestsName = std::to_string(std::get<1>(info.param));
> -
> -	return roleName + "_" + numRequestsName;
> +	return roleToString(std::get<0>(info.param)) + "_" +
> +	       std::to_string(std::get<1>(info.param));
>  }
>  
>  /*

-- 
Regards,

Laurent Pinchart