HiveBrain v1.2.0
Get Started
← Back to all entries
patternbashMinor

Script to download sequentially named files, rename them, and delete smaller files

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scriptdeleteandrenamenamedfilessmallerdownloadthemsequentially

Problem

I've written a little script to download sequentially named files, rename them, and delete files smaller than an certain number of kilobytes. I came up with this but I'm not too happy. Any advice for me?

#!/bin/sh
echo
echo -n "Start file #:"
read start
echo
echo -n "End file #:"
read end
echo

wget --progress=bar:force http://www.mybox.com/pictures/show_pic.asp?src_id={$start..$end} 2>&1 | progressfilt

sleep 1

for file in *.asp
do
    mv "$file" "${file%.asp}.jpg"
done

find * -maxdepth 1 -name "*.jpg" -size -300k -delete

echo "tasks completed...exiting"
exit

Solution


  • sh is not bash. This shouldn't be a problem for your script though, because I don't think there are any bashisms in there.



  • Use More Quotes™. If there had been another parameter in the URL you would have needed an ampersand (&) in there, and that's the special character to run a command in the background. That's one of many ways to trip up if you don't comment basically any non-trivial string.



  • You don't need to sleep, because the wget commands are synchronous.



  • That said, you might get a result much faster if you run them in parallel, by doing several wget […] & commands followed by a wait.



  • You don't need to exit at the end of a shell script. That's only useful if you want to exit prematurely or with a different exit code from the previous command.



  • If you use Bash you can use read -p "Start file #: " start etc. to avoid most of the echoing.



-
However, it is much more common to make shell scripts take parameters instead of being interactive, making the script even simpler:

start="$1"
end="$2"


  • Use set -o errexit -o nounset (and -o pipefail if you're really in Bash) to make sure you catch any common bugs early.



  • I'm not sure what progressfilt does, but you can suppress the wget progress bar by using --quiet.



  • To be done slightly faster you should swap around the deleting and the renaming.



  • Rather than find * you should use find . to avoid exceeding the maximum argument length. This is a good practice in general, but will only really break with lots of files.



  • One very common feature of *nix scripts is that they are silent unless there's something important to report. Therefore, I'd remove the last echo.



Your script should now be 9 SLOC, and clean as a whistle!

Code Snippets

start="$1"
end="$2"

Context

StackExchange Code Review Q#132962, answer score: 2

Revisions (0)

No revisions yet.