current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Unix & Linux Stack Exchange is a question and answer site for users of Linux, FreeBSD and other Un*x-like operating systems.. It's 100% free, no registration required.
up vote 1 down vote favorite

I'm trying to write a code that moves files to a .junk directory using the built-in environment variables. My goal is that all a user would have to do is type sh junk filename1 filename2 filename3... and the script would use $1, $2, $3... to move the files. Unfortunately I can't quite get the counter loop to work correctly.

#!/bin/s

if [ $# = 0 ]
then echo "Usage: junk -l | -p | { filename }*"

elif [ $1 = "-l" ] && [ $HOME/.junk ]
     then ls $HOME/.junk

elif [ $1 = "-p" ] && [ $HOME/.junk ]
     then rm -r $HOME/.junk

elif [ $1 = "$@" ] && [ $HOME/.junk ]
     then echo $#

i=1
s="$"

while [ $i -le $# ]
do
 echo $s$i
 mv $s$i .junk/$i
 i=$(($i+1))
done

fi
share|improve this question
1  
What are "built-in environment variables"? Which shell are you using /bin/s? –  Anthon Apr 28 at 21:36
    
That is terrible code. $1 should always be quoted: "$1". What is [ $HOME/.junk ] supposed to be/do? And [ $1 = "$@" ]? Why does while [ $i -le $# ] start with i=1 instead of i=0? What is supposed to happen with a single file argument? mv $s$i is probably supposed to be an (unquotes, of course) indirection which would have to be done with eval or (better) "${!i}"... The counter loop really is the least of your problems. –  Hauke Laging Apr 28 at 21:44
    
Why are you using a loop in the first place? All you really need is mv "$@" .junk. Could you explain what your script is doing and why you want the loop? –  terdon♦ Apr 28 at 21:52
    
@terdon there is one barely possible chance that mv "$@" ./junk will present argument list size problems, but that would take a lot of junk. –  mikeserv Apr 29 at 0:07

1 Answer 1

up vote 0 down vote

Some immediate problems:

  1. Use getopts for argument processing or have a reason why not.
  2. If the script is given more than 1 argument, the "$@" in the third elif will blow it up.
  3. Use a for f in "$@" loop instead of a while loop.

Why?

  1. What happens if you type sh junk --help ... or some more interesting mv option?
  2. QED
  3. Simpler => Better
share|improve this answer
1  
for f in "$@" ; do is hardly simpler than for f ; do which is the same thing when you're iterating on positionals. Also while and until allow conditionals - as many as you can put in a compound command list, thought I don't think our asker is making good use of it. One I always use for positionals is while ${1+:} false ; do which is pretty simple. It also gives me the freedom of skipping iterations with shift as needed. –  mikeserv Apr 29 at 18:15
    
@mikeserv Good comment, but it would be a better answer. Just add one sentence at the top pointing out that attempting to treat positional parameters like an array is the wrong idea. It might actually be useful to some other bash beginner and I'll upvote it past my grumpy answer. –  Scott Leadley Apr 29 at 18:32
    
Positional parameters are an array. The shell positional $@array is the only fully portable shell array there is. And with its special IFS handling its really the only one you need. –  mikeserv Apr 29 at 19:29

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.