5
\$\begingroup\$

This is a pretty basic bash script (3.2 on Mac). I am downloading 584 images from a site in order to create an album.

#!/bin/bash
urlFirst="http://150.216.68.252:8080/adore-djatoka/resolver?url_ver=Z39.88-2004&rft_id=http://150.216.68.252/ncgre000/00000012/00011462/00011462_ac_0"
urlSecond=".jp2&svc_id=info:lanl-repo/svc/getRegion&svc_val_fmt=info:ofi/fmt:kev:mtx:jpeg2000&svc.format=image/jpeg&svc.level=6"
for i in {1..584}
do
 printf -v j "%03d" $i
 url=$urlFirst$j$urlSecond 
 wget $url -O $j".jpeg"
done

What improvements should be made to this?

I understand that I could remove the variables in most cases but for debug purposes it was helpful to have them, as I could add basic echo statements.

I'm not very well versed in Bash scripting though (lots of basic scripts) - any improvements or best practices from Bash I'm missing would be appreciated.

200_success
145k22 gold badges190 silver badges478 bronze badges
asked Dec 24, 2015 at 16:07
\$\endgroup\$
4
  • \$\begingroup\$ You need to quote the $url variable (wget "$url" ...), otherwise the first & will send the wget command into the background without the complete url. See unix.stackexchange.com/questions/171346/… \$\endgroup\$ Commented Dec 24, 2015 at 16:10
  • \$\begingroup\$ @glennjackman do I? it... is running right now and downloading images successfully. Or is this a "you really should do this best practice" type of thing? \$\endgroup\$ Commented Dec 24, 2015 at 16:11
  • 1
    \$\begingroup\$ OK, that's my error. I was thinking x="foo&bar"; echo $x would act the same as echo foo&bar. I forgot that the shell does not interpret things like ; and & unless you use eval. But I would encourage you to read the link I provided. I would definitely write wget "$url" -O "$j.jpeg" with all variables quoted. \$\endgroup\$ Commented Dec 24, 2015 at 16:19
  • \$\begingroup\$ I only downloaded the first image. Looks pretty cool. \$\endgroup\$ Commented Dec 24, 2015 at 16:23

2 Answers 2

3
\$\begingroup\$

I would write it this way:

#!/bin/bash
url_template="http://150.216.68.252:8080/adore-djatoka/resolver?url_ver=Z39.88-2004&svc_id=info:lanl-repo/svc/getRegion&svc_val_fmt=info:ofi/fmt:kev:mtx:jpeg2000&svc.format=image/jpeg&svc.level=6&rft_id=http://150.216.68.252/ncgre000/00000012/00011462/00011462_ac_%04d.jp2"
for i in {1..584}; do
 wget -O $(printf '%03d.jpeg' $i) "$(printf "$url_template" $i)"
done

Specifically,

  • At first glance, I thought that $urlFirst and $urlSecond were two URLs, until I figured out what was happening.
  • If there is another leading 0, then %04d is more appropriate than %03d.
  • The web server isn't going to care about the order of the parameters in the query string. For the benefit of humans, though, it would be nicer to put the parameter that changes either first or last.
  • Fewer variables is nice. I do everything by interpolation. You can get a feel for what the wget command does, without tracing which variable came from where.
answered Dec 24, 2015 at 17:24
\$\endgroup\$
2
\$\begingroup\$

Nicely written. I especially like the printf -v trick to write to a variable directly, as opposed to j=$(printf ...). I didn't know this feature of printf, very cool, so thanks for that.

Speaking of which, j is a poor name, like most single-letter variable names. filename would have been better, and the code would have been instantly easier to read.

ShellCheck gives a warning for the wget $url, suggesting to double quote to prevent globbing and word splitting. For example if the URL contained a space, the script wouldn't work. Although this is not a real concern in your example, I suggest to take the good advice and double-quote it anyway.

The double-quoting of ".jpeg" is a bit odd. You didn't need to quote that.

answered Dec 24, 2015 at 17:08
\$\endgroup\$
1
  • \$\begingroup\$ The printf trick was from a Stack Overflow question. Can't take credit for that :) \$\endgroup\$ Commented Dec 24, 2015 at 17:08

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.