bash unset array loops GARBAGE

strongdrink · 06-27-2011, 03:47 PM

Hello everyone.. i dont really know how to explain this.. because i dont really know EXACTLY what is wrong.. so here is my code..

Code:

#loop until there are no more files
while [ ${#files[@]} -gt 0 ]; do
 
  for num in `seq 1 ${#files[@]}`; do
 
    wurd=`tail -4 ${files[$num-1]}`

    if [[ "$wurd" == *OK* ]]; then
   
      echo "[  OK  ] ${files[$num-1]}"
      unset files[$num-1]
 
    elif [[ "$wurd" == *FAILURE* ]]; then
  
      echo "[ FAIL ] ${files[$num-1]}"
      unset files[$num-1]
  
    fi
  
    #echo the file name
    echo ${files[$num-1]}
 
  done
 
  #wait a second
  sleep 1
 
done

Everything works.. but when it has to unset.. it breaks.. it doesnt give an error though.. it just jams the program..

thanks!

grail · 06-27-2011, 07:26 PM

Please use [code][/code] tags to display your code.

Quote:

it breaks

You might need to explain this further? You say there is no error message so how do you know it is broken?

I am struggling to follow the logic as well. It appears that OK or FAILURE both remove items and as your outside
loop is looking for when all have been unset I am guessing these are the only 2 options, so why not just process
the loop from start to finish and echo the corresponding output, ie no need to unset that I can see.

David the H. · 06-28-2011, 12:46 AM

I agree with the above. code tags will make it more readable, and using a decent indentation style is even better. Also, the line numbers make it hard to simply cut&paste to a file for testing.

In addition, a short explanation of what you're really trying to accomplish may help clarify things.

There are also a few issues with the code I want to mention, outside of your main problem.

1)

Code:

 while [ ${#files[@]} -gt 0 ]; do

In bash, the old [ test command can be better replaced with the [[ keyword. But in this case you're doing an arithmetic comparison, so ((..)) is recommended.

2) This whole outside while loop appears to be completely unnecessary anyway, as the inner loop will only output existing entries.

3)

Code:

for num in `seq 1 ${#files[@]}`; do

Don't use seq. Use brace expansion instead.

But actually, there's an even easier way to loop through an array. ${!array[@]} will give you a list of all existing index elements, so you can loop over only entries that exist, rather than a simple number list. It's particularly good for sparse arrays.

4) $(..) is highly recommended over `..`

5) Always quote your variable expansions, unless you want word-splitting to occur.

6)

Code:

if [[ "$wurd" == *OK* ]]; then

I would replace the if construct with a case construct here. That way you only do a single evaluation of the input.

So taking all this into account, this is how I would write it. But note the comment on the echo command at the end:

Code:

#!/bin/bash

for num in ${!files[@]}; do

     wurd=$( tail -4 "${files[num]}" )

          case "$wurd" in

               *OK* )
                    echo "[ OK ] ${files[num]}"
                    unset files[num]
               ;;

               *FAILURE* )
                    echo "[ FAIL ] ${files[num]}"
                    unset files[num]
               ;;

          esac

     #This will only print a blank line, since you just unset the entry above.
     echo "${files[$num]}"

     #wait a second
     sleep 1

done

strongdrink · 06-28-2011, 09:01 PM

I use vim with line numbers turned on sry :S and it was indented.. i dunno why it took the indents away...

1) I am using a while loop because the files are being generated AS the script is running
2) "
3) Ok thanks, I just read on a forum that i should use seq..
4)
5)
6) Ok thanks

David the H. · 06-28-2011, 11:02 PM

So did this help you fix your problem? If it did, you should tell us about it, so we what worked, and any future readers can also be helped by it. Then mark the thread as solved.

When HTML is rendered it condenses all contiguous whitespace down to single spaces, and leading whitespace is removed. Special tags or characters like <blockquote> or   must be used in the HTML code to preserve it.

But since this forum has disabled the direct use of html in posts, you have to use the bbcode code tags instead, which the server will then convert into a blockquote (or more precisely a "pre" div) before it sends the page out.

As for things like seq, you'll find a lot of coding suggestions out there that are outdated or not generally recommended. Many shell coders come from the old unix world and used shells that don't have all the features bash does. They may not know about the other choices, got bad advice themselves in the past, are thinking in terms of cross-platform support, or are simply too ingrained in their old ways to change.

But unless you need portability to systems that don't have newer versions of bash installed, then there's no reason not to use all the goodies it has to offer. It's better in general to use built-in features than to rely on external tools whenever possible.

Anyway, don't take it as any kind of criticism. My only desire is to help point out good practices so you can improve your scripting.

I've been finding Greg's Wiki to be a real treasure trove of bash info. I've been reading a lot of it recently and it's really helped my understanding.

grail · 06-28-2011, 11:23 PM

+1 on Greg's Wiki

strongdrink · 06-29-2011, 07:37 AM

Ok I'll post if it worked as asap.

strongdrink · 06-29-2011, 07:56 AM

Ok it's working! The problem was the for loop..

Code:

  for num in `seq 1 ${#files[@]}`; do

because it was still reading elements that i unset

so, like "David The H." said, i changed it to

Code:

  for num in ${!files[@]}; do

and its working swell now

thanks everyone!

P.S. i changed a lot of my sloppy scripting as well :S