I kind of like shell scripting. It’s quirky, frequently ugly, but it’s damned useful. Like Perl, it annoys me when I see things which could be written in a better manner. One persistent example is abusing the if statement.
if [ "x$machine" = "x" ] then echo "not in ~/.machines adding" echo $1 >> ~/.machines source ~/.bash_aliases fi
What’s the problem here? That little “x” in the test. It’s not necessary and hasn’t been since the late 80’s. There used to be a parsing bug in shells which meant that an empty argument got removed. So you couldn’t say [ "$machine" = "" ]
because you’d end up comparing $machine
to ]
.
But this got fixed, a long time ago. Really. Unless you’re working on an aging System III box, you shouldn’t worry about this, it’s just Cargo cult programming.
So the test can be just [ "$machine" = "" ]
. That’s better. But not good enough. Testing for an empty string? The test command (aka [
) provides a -z
operator for that! So goes down to [ -z "$machine" ]
.
In this particular case, we’re explicitly targetting bash, though. One of the nice features of modern POSIX shells like bash is that they provide conditional expressions, which have different parsing rules. So you can say [[ -z $machine ]]
and not worry about the lack of quotes.
Lastly, I should point out that I missed off the first line of that extract.
machine=`grep $1 ~/.machines`
With that in place, you can see that we’re not actually using $machine
anywhere in the function. It’s purely in place for the if
statement. Which means you can simplify this even further. if
takes a command as it’s argument. So why not just give it the grep command?
if grep -q $1 ~/.machines then echo "not in ~/.machines adding" echo $1 >> ~/.machines source ~/.bash_aliases fi
I had to add the -q flag to shut grep up; we’re only interested in the return code, not the output.
Does this matter? Probably not much. But like all programming languages, it’s a lot easier to read when you use it idiomatically.