help-bash
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Organising conditions


From: Greg Wooledge
Subject: Re: Organising conditions
Date: Mon, 2 Aug 2021 12:07:26 -0400

On Mon, Aug 02, 2021 at 03:40:24PM +0000, hancooper wrote:
> It is a generic question.  I got into it as am populating an array of files, 
> checking
> if they exist or are redundant.
> 
>     for file in "$@"; do
>       file_redundant=false
>       for fde in "${flist[@]}"; do
>         [[ $fde == $file ]] && file_redundant=true
>       done
>       [[ (-f "$file") && (! $file_redundant) ]] && flist+=("$file")
>     done

OK.  You're trying to remove the duplicates from a list, while retaining
the original order of the list.  Also, you're checking whether each list
element is a regular file, and excluding those which aren't.

Here's how I would approach it:

declare -A seen
flist=()
for file in "$@"; do
    [[ ${seen[x$file]} ]] && continue
    [[ ! -f $file ]] && continue
    flist+=("$file")
    seen[x$file]=1
done

There are obviously other ways it can be done, but:

1) Your algorithm searches the entire existing list every time a new
   item is being considered; it doesn't even break out of the loop when
   the item is seen.  This makes your algorithm run with O(n^2) time
   complexity.  With the associative array, the complexity is O(n).

2) Your check of the string $file_redundant is incorrect.  You're assigning
   the values "true" or "false" to it, but you're not actually checking
   for those values.  Instead, you're checking whether the string is
   empty or non-empty.  The string is always non-empty, which is why your
   check is awlays failing.

3) I used the "x hack" with the associative array keys, because at this
   time, it's not possible to store all possible values in an associative
   array in bash.  Certain values (the empty string, @ and *) are treated
   specially, and two of those are valid filenames.

4) I really dislike long, complex "if" conditions.  I find them hard to
   read and debug.  I prefer to use separate checks most of the time.
   Another advantage of checking each condition separately is that you
   can add unique warning/log messages to each case, which further helps
   debugging.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]