help-bash
[Top][All Lists]
Advanced

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

Re: Organising conditions


From: hancooper
Subject: Re: Organising conditions
Date: Mon, 02 Aug 2021 17:06:34 +0000

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, August 2, 2021 4:07 PM, Greg Wooledge <greg@wooledge.org> wrote:

> 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

Have done some tests and looks that I can also use seen[x-$file] as a key.


> 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]