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

[Aaron Lauterer <a.lauterer@proxmox.com>]

On  2024-09-09  16:02, Fiona Ebner wrote:
> 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.
> 

thanks! sounds like a good idea. I will wait a bit before sending a v5.


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



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