Re: [pve-devel] [PATCH common v4 1/7] tools: add check_list_empty function

[Fiona Ebner <f.ebner@proxmox.com>]

Am 29.07.24 um 13:55 schrieb Aaron Lauterer:
> In some situations we don't want a total empty list. I opted for a
> dedicated function instead of integrating it as error in the
> `split_list` function. It is used in many places and the potential
> fallout from unintended behavior changes is too big.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> Tested-By: Stefan Hanreich <s.hanreich@proxmox.com>
> Reviewed-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
> changes since: v3: none
> v2: newly added
> 
>  src/PVE/Tools.pm | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
> index bd305bd..f796bd0 100644
> --- a/src/PVE/Tools.pm
> +++ b/src/PVE/Tools.pm
> @@ -718,6 +718,14 @@ sub split_list {
>      return @data;
>  }
>  
> +sub check_list_empty {
> +    my ($list) = @_;
> +    if (scalar(PVE::Tools::split_list($list)) < 1) {
> +	return 0;
> +    }
> +    return 1;
> +}

This can be very confusing IMHO. Intuitively, I'd expect the expression
check_list_empty($list) to be truthy if $list is empty. I'd rather call
it list_not_empty. But looking at the caller you introduce later, it
might be better to avoid the double negative, flip the truth table and
call it list_is_empty.

> +
>  sub trim {
>      my $txt = shift;
>  


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel