1
\$\begingroup\$

I wrote a short bash script for a completion check of downloading images.

goal of this code (Added)

I have some downloading content directories. Each of them has a script for downloading images(tmp.sh), for example, the following one:

wget -nc ¥
  http://www.example.com/001.jpg ¥
  http://www.example.com/002.jpg ¥
  ...
  http://www.example.com/123.jpg ¥

I want to know which wget completes downloading all or failed.

If you have any good ideas, please tell me them. Thank you.

done.sh

#!/bin/sh

# if a directory name is assigned

if [ $# -eq 1 ]; then

  a=`ls -1 $1/*.jpg | wc -l`
  b=`grep jpg $1/tmp.sh | wc -l`
  if [ $a = $b ]; then
    echo $1
  fi
  exit
fi

# if some directory names are assigned

if [ $# -gt 1 ]; then
  for dir1 in "$@"; do
    ./done.sh $dir1
  done
fi

# if directory names are not assigned

if [ $# -eq 0 ]; then
  ls -d */ | xargs ./done.sh
fi
```
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Good

  • good indentation
  • somewhat helpful comments
  • good to do the ; then bit on the same line as the if

Suggestions

  • use double brackets for conditionals
  • try shellcheck (It will tell you to put quotes around your variable substitutions.)
  • put your arguments into named variables so you don't keep have to referring to $1
  • put the 3 sections into if...elif...else structure to make clear that you're only going to go into one of them
  • check for the existance of ./done.sh at the beginning
  • explain what the goal of the script is somewhere.
\$\endgroup\$
3
  • \$\begingroup\$ Thanks for the great suggestions. I forgot the programming fundamental techniques in shell programming. I'll correct my code and paste it below the original code. \$\endgroup\$ Commented May 30, 2021 at 4:04
  • \$\begingroup\$ @HaruoWakakusa You may want to read What I can and cannot do after receiving answers before you do that. As it is one of the things that you can't do under this site's rules. \$\endgroup\$
    – mdfst13
    Commented May 30, 2021 at 4:10
  • 1
    \$\begingroup\$ Yes, I'll add only the purpose of this code. \$\endgroup\$ Commented May 30, 2021 at 4:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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