Help improve my scripting skills

August 29, 2007 6:55 AM

I wrote a shell script for an acquaintance that renames Macs (OS 10.4) based on their existing names. It works well, but I'm convinced my code could be more efficient. Where can I make improvements?

First, some background. The old naming scheme was [Building]_[CPU speed]_[Model]_[number]. The length of the names was cumbersome, so they decided to shorten them to [abbreviated building name]-[model]-[A/B][number], where A=1.42GHz and B=1.25GHz. For example, if the old names are

Berkeley_1.25Ghz_eMac_01
Washington_700MHz_iMac_01

the new names should be

berk-emac-B01
wash-imac-01 (they only have 700Mhz iMacs)

This is the bash script I came up with (with comments), that gets run as root on each computer:

# get the current computer name
NAME=`scutil --get ComputerName`

# split the name into its component parts
BLDG=`echo $NAME | awk -F_ '{print $1}'`
CPU=`echo $NAME | awk -F_ '{print $2}'`
MOD=`echo $NAME | awk -F_ '{print $3}'`
NUM=`echo $NAME | awk -F_ '{print $4}'`

# replace full building name with abbreviation
newBLDG=`echo $BLDG | sed \
-e 's/Berkeley/berk/' \
-e 's/Washington/wash/'`

# make model lowercase
newMOD=`echo $MOD | tr [:upper:] [:lower:]`

# replace cpu speed with letter (or no letter for 700MHz)
newCPU=`echo $CPU | sed \
-e 's/1.42GHz/A/' \
-e 's/1.25GHz/B/' \
-e 's/700MHz//'`

# reassemble name from new parts
newNAME=$newBLDG-$newMOD-$newCPU$NUM

# assign the new name to the computer
scutil --set ComputerName $newNAME
scutil --set LocalHostName $newNAME

posted by pmbuko to computers & internet (11 comments total)

not sure it's relevant here, but commands that are often useful in cases like this, and perhaps not that well-known, are "basename" and "dirname". they save you from having to use sed to split paths.
posted by andrew cooke at 7:10 AM on August 29, 2007

You could do most of that in just one instance of Awk. I'll come up with an example.
posted by cmiller at 7:14 AM on August 29, 2007

Is that actually slow, or is this theoretical efficiency? Because that is pretty readable and commented, and replacing it with a one liner will not be.
posted by smackfu at 7:25 AM on August 29, 2007

Since this is only going to run once, efficiency is not really that important. Premature optimization is the root of all evil. But I think it's good you want to learn how to script better. I'll make some stylistic comments as well.

Using $(...) is better than `...` for a couple of reasons. First, it looks clearer and is harder to get confused with parentheses. Second, it means you can nest command substitutions if you want. Third, it means you don't have to have a confusing extra level of backslashes if you're using backslashes.

You can use cut -d _ -f 1 instead of awk -F_ '{print $1}'. Most efficiently would be to do it all in one go using perl.

You can combine setting BLDG and newBLDG into one command substitution.

Finally, you better be damn sure that all of the computers you run this on fit the pattern you're expecting, or you will get some strange results.
posted by grouse at 7:27 AM on August 29, 2007

smackfu and grouse have it right. This is pretty darned good as it is.

I removed some of your assumptions, but some still exist (like, cities are at least 4 chars long).

echo Berkeley_700Mhz_eMac_01 | 

  awk -F_ '{ print substr(tolower($1), 0, 4) "-" tolower($3) "-" ($2 == "700MHz" ? "" : ($2 == "1.42Ghz" ? "A" : ($2 == "1.25Ghz" ? "B" : $2))) "-" $4}'

posted by cmiller at 7:50 AM on August 29, 2007

Heh, cmiller. I think that's the point when it's time to start using a separate file to store your awk program rather than feeding it in over the command line. Changing that later would be a big pain.
posted by grouse at 7:59 AM on August 29, 2007

By efficient, I simply meant more concise. Looking back at it, my existing code is pretty readable and easy for the non-coder I wrote it for to decipher. cmiller wins for conciseness with his one-liner, and grouse gets points for his tips. Thanks!
posted by pmbuko at 8:24 AM on August 29, 2007

Agreed. I think mine is a good example of what /not/ to do.
posted by cmiller at 8:27 AM on August 29, 2007

eval $(echo $NAME | awk -F_ '{print "BLDG=" $1 ";CPU=" $2 ";MOD=" $3 ";NUM=" $4 }')
posted by [@I][:+:][@I] at 10:03 AM on August 29, 2007

eval $(scutil --get ComputerName | awk -F_ '
{ print "BLDG=" tolower(substr($1,1,4)) ";CPU=" $2 ";MOD=" tolower($3) ";NUM=" $4 }
')

takes care of a lot of it
posted by [@I][:+:][@I] at 10:14 AM on August 29, 2007

ok, sorry last one. This is how I would do, I think it's pretty readable:

NAME=$(scutil --get ComputerName | awk -F_ '{
if( $2 == "1.42GHz" ) { x = "A" } else if( $2 == "1.25GHz" ) x = "B"
print tolower(substr($1,1,4)) "-" tolower($3) "-" x "-" $4
}')
scutil --set ComputerName $NAME
scutil --set LocalHostName $NAME
posted by [@I][:+:][@I] at 10:22 AM on August 29, 2007