Sometimes one gets to re-write them to add new features, and some re-factoring brings great relief.
See this function wrapper to run a command more than once in case it fails the first time. (Don't get distracted on thoughts of the validity of this approach).
do_cmd() { retval=0 for i in `seq 1 2`; do cmd 2>/dev/null $@ retval=$? if [ $retval -eq 0 ]; then break fi sleep 1 done return ${retval} }
Can we express that function any more concisely or correctly?
The first thing that comes to mind is
local retval=0
...but that is closely followed by the wonder of
`seq 1 2`
which emits 1 2
and so that line could become:for i in 1 2
At this point it becomes very obvious that the function will execute
cmd
up to twice, with a 1 second delay if it fails the first time. Any bash coder given that description could come up with a 1 liner; but let's approach it by degrees.This chunk:
cmd 2>/dev/null $@ retval=$? if [ $retval -eq 0 ]; then break fi
becomes:
cmd 2>/dev/null $@ retval=$? test $retval -eq 0 && break
but we don't need to remember retval if we return right away, so this might become:
cmd 2>/dev/null $@ && return retval=$?
(we still remember
retval
so that we can return the last failure code if the loop exits.But who needs a loop for 2 iterations? How about this:
do_cmd() { cmd 2>/dev/null $@ && return sleep 1 cmd 2>/dev/null $@ }
Which is pretty clear. But compare that to a fresh re-write based on the original description above:
do_cmd() { cmd "$@" || sleep 1 && cmd "$@" } 2>/dev/null
I also added in the missing quotes around
$@
and added the stderr redirector to the function body definition.The function will exit with either 0 because the first invocation succeeds, or it will return with the exit code of the second invocation 1 second later - presuming
sleep
doesn't fail.If we are afraid that sleep might fail, we could make the second invocation not dependant on the success of the sleep command:
do_cmd() { cmd "$@" || { sleep 1 ; cmd "$@" ; } } 2>/dev/null
Of course this doesn't quite match the original function because any stderr resulting from a failure in executing the sleep command would also be hidden.
Heh, for a moment the naming of do_cmd() made me think, "Gosh, did I write that at some point?". But I'm hoping I didn't, based at least on the lack of "local" for retval, the loop structure, and the return value checking style.
ReplyDeleteThis reminds me of the discussions around more "Pythonic" ways of achieving a goal in Python. A true Bashista would prefer the refactored do_cmd(), but I've seen plenty of people write do_cmd() in the same fashion as the original.
Really, though, the key issue with refactoring Bash scripts is that they rarely have much in the way of test harnesses or documentation. So it's sometimes tricky to figure out whether your refactorings have been beneficial or not :-|