Wanna critique my bash script?

[chris27]

Hi all. I grabbed a script from O'Reilly's Sed and Awk manual. However, the author wrote the script in Sh and I re-wrote it and modified it in BASH. However, Im not a BASH guru and I would love for some people to give me suggestions on how to make it better... so here it is.



runsed

#applies the sed commands found in the file secscr to the] #filename of choice.


#!/bin/bash

filename=$1
if [ -z $filename ]; then
echo -e "Usage: runsed <filename>."
exit 1
elif [ "$filename" = "sescr" ]; then
echo -e "Not editing secscr!"
exit 1
else
sed -f sedscr $filename > $filename.tmp
if [ cmp -s $filename $filename.tmp ]; then
echo -e "File not changed.\n"
else
mv $filename $filename.bak
cp $filename.tmp $filename
fi
rm -f $filename.tmp
echo -e "Done!"
fi


thats it.. any comments, suggestions, editorials, perusals etc...?

[jetblackz]

To be a good programmer, you want to comment your code and stick to a coding style. Add 2 spaces at the beginning of each line and 2 more spaces for the next "level" of code, starting from outside in.

Otherwise, it looks good to me.

[chris27]

Coding style requires 2 spaces on each line up front?
For example, instead of...

if [ -z $1 ]; then
--echo "blah"

but

--if [-z $1 ]; then
----echo "Blah"

(I actually did indent in my 'second level' of code but it appears to have been lost when I send in the post. I also indented 2 spaces here on the second level, lets see if it sticks this time)

If the second example is correct then I understand what you were speaking of. (I indented 2 spaces before the "if") That is something I was unaware and I think you for your comments!

**ok, my indents are not sticking when I send in the code on this board so I have used the "-" charcter to symbolize a space.

[bwkaz]

Regarding the indentation, you can really use however many spaces you want. It shouldn't matter.

However, to get the boards (or actually, our browsers) to keep them, you can insert code tags around your, well, code. They look like [ code] and [ /code], except without the space between the [ and the rest of the tag.

It'll end up looking like this:

#!/bin/bash

# This is some sample code You might want to edit your first post (the spaces will still be there) and enclose the code in code tags.

Something I see in the code itself is, you'll probably want to put more quotes around things like $filename, $1, etc., so that paths with spaces in them will work. You'll probably also want to check for if the file passed as $1 actually exists and is readable. You will also want to make your magic sed script file's name consistent (in the messages, in the test, and in the sed command). Finally, you don't need the -e on the echo command unless you're using escape codes (like \n for newline), and you don't need to -- echo automatically appends a \n unless you pass the -n option as well. Perhaps change it to look like:

#!/bin/bash

# Quotes around $1 so that paths with spaces can be used
filename="$1"

if [ -z "$filename" ]; then
echo "Usage: runsed <filename>."
exit 1
elif [ "$filename" = "sedscr" ]; then
echo "Not editing sedscr!"
exit 1
elif ! [ -r "$filename" ] ; then
echo $filename does not exist or is not readable.
exit 1
else
sed -f sedscr "$filename" > "$filename.tmp"

if [ cmp -s "$filename" "$filename.tmp" ]; then
echo "File not changed."
echo ""
else
mv "$filename" "$filename.bak"
cp "$filename.tmp" "$filename"
fi

rm -f "$filename.tmp"
echo "Done!"
fi

[chris27]

Thanks Bwkaz, that was an extremely helpful reply!


One question about your code though..
Im not at a linux machine now so I cant check it out by myeslf but in the second elif statement, i noticed the following code.

elif ! [ -r "filename" ] blah blah

Is it ok to put the "!" outside of the brackets in this case?
I have always included it inside the brackets but if both are possible, is there a "standard" or "accepted" stlye that is in popular use?


Thanks again!

[bwkaz]

Outside or inside will, AFAIK, work. I don't know whether there's an accepted "standard" or whatever, I just do whatever I want when I write it.

The difference is that with the ! outside, the if command is doing the logical negation. With it inside the bracket, the negation is done by either the test binary ([ is a symlink to test, usually) or the bash test/[ function thingy. With either, the effect is the same.