Factor a shell script

When factoring scripts into functions you soon notice the functions are separately testable from the command line. For instance, I often see questions in the shell forums like: _"I've got this script which works when … except when … "_. Too often the problem boils down to some shell or file-system detail which could have been isolated in a separately testable function.

In such cases, I believe it's a positive learning experience to home in on the simple facet and build a solution for the low-level detail. With practice and experience, you'll be discovering natural factors of the environment which should lead to a fair degree of re-use.

When it's time to name a function, think of action verbs which describe what is happening rather than how. Occasionally use verb-noun pairs. I'll offer no suggestions on name conventions, other than notice I use both underscore (\_) and camelCase notations for multi-word function names.

What is syntactic noise? Any highly-repeated syntax element of the programming language. The easiest one to spot is the repeated use, or use at all of the append to file syntax ( >> ). Where you see this in a script, you've identified the place to factor the script into functions. If you're not familiar with the so-called reverse engineering practice of factoring, it's collecting small code segments into separate components. In this case, shell functions.

In general, I'm proposing a metric for shell programming. When I've concluded a thorough review of the RE shortcuts: alphanum, etc… , I'll propose a weighting vector for each class, from 0 to 1, which when applied to the body of any code block returns it's synoise, the weighted score.

Eliminating temporary files will lead to simpler functions, if not better use of system resources. People often mis-interpret this goal as improving machine performance. The more important reason is improving human understanding. The Unix &reg#; pipe was invented as a means to connect smaller, well-specified processes.

Uses of a temporary file, when a pipe would suffice, exhibits the designer's lack of confidence in what is a well-specified component. A well-constructed pipeline, with meaningful functions, is more easily read and understood than a thicket of redirections to a temporary file.

Temporary variables with well-chosen names may be attractive, but as we will see in this example, two variables are really used as one, and might even have been written into a simple function, not for any reason in this code as such, but as generic means of identifying the server.

This exercise concludes by introducing an ENVIRONMENT variable for a log, or message file. This does two things: it allows users to define their own log file, or possibly make the logs execution-time dependent, or it may throw into question the need for this particular log file at all.

The use of arguments supports the ability to factor the problem into a single high-level command. This high-level command may become the focus of a wider generalization where system names, log file conventions, and in this case email-recipients are decided, and not written into operational code.

Concise functions needn't re-name positional parameters.

This script's author says it's successfully used in a crontab at 10 minute intervals.

In it's un-factored form, it offers too-great a temptation to copy the entire thing, make a few changes, and thus proliferate natural components in an un-maintainable fashion. Not that evolution is bad, but with a functional approach, each maintainer may be challenged to discover ways to keep the general code working while introducing local distinct criteria. I'll point at a few possibilities in the conclusion.

HSTNM=$(hostname)-ebs505 
MAILLST="thom email@removed email@removed" #<<< Use login names 
seperated by spaces 
OSLVL=$(oslevel -r) 
x=`df -kt |grep 9[4-9]%` 
if [[ -n ${x} ]] 
then 
echo "\n$(date) hostname=$HSTNM,$OSLVL NEEDS ATTENTION:" > /ebs/misc/tmp/ebsdfwa 
rn.out 
echo "\n\r\rWARNING these file systems are approaching being full on hostname=$H 
STNM,$OSLVL:\r" >> /ebs/misc/tmp/ebsdfwarn.out 
echo "\r" >> /ebs/misc/tmp/ebsdfwarn.out 
echo "Filesystem 1024-blocks Used Free %Used Mounted on\r" >> /ebs/ 
misc/tmp/ebsdfwarn.out 
echo "${x}\r" >> /ebs/misc/tmp/ebsdfwarn.out 
echo "\r" >> /ebs/misc/tmp/ebsdfwarn.out 
echo ">>>>>>>>>> THIS CAN POTENTIALLY CAUSE LOSS OF DATA <<<<<<<<<\r\r" >> /ebs/ 
misc/tmp/ebsdfwarn.out 
find /tmp -xdev -size +2048 -ls |sort -r +6 >> /ebs/misc/tmp/ebsdfwarn.out 
mail -s "!!!!! CHECK %USED FOR FILE SYSTEMS ON hostname = $HSTNM,$OSLVL !! 
!!!" ${MAILLST} < /ebs/misc/tmp/ebsdfwarn.out 
echo "END WITH email $(date) on hostname=$HSTNM,$OSLVL" 
exit 1 
fi 
date 
find /tmp -xdev -size +2048 -ls |sort -r +6 
echo "END WITHOUT email $(date) on hostname=$HSTNM,$OSLVL"

ebs_host ()  { $(hostname)-ebs505,$(oslevel -r); }
syswarn () 
{ 
    echo
    echo "$(date) hostname=$(ebs_host) NEEDS ATTENTION:" 
    echo 
    echo "WARNING these file systems are approaching being full on hostname=$1:"
    echo 
    echo "Filesystem 1024-blocks Used Free %Used Mounted on\r"
    echo $1
    echo 
    echo ">>>>>>>>>> THIS CAN POTENTIALLY CAUSE LOSS OF DATA <<<<<<<<<" 
    echo
    find /tmp -xdev -size +2048 -ls |sort -r +6
}
# shifted () { shift; echo $*; } -- now use "shift; echo" idiom, see below
warn_status () 
{
    echo "END WITH email $(date) on hostname=$1" 
    mailsub="!!!!! CHECK %USED FOR FILE SYSTEMS ON hostname = $1 !!!!!"
    syswarn "$1" | tee /ebs/misc/tmp/ebsdfwarn.out | mail -s "$mailsub" $(shift; echo $*)
}
ok_status () 
{
    : ~ 
    :    reports to standard out
    date 
    find /tmp -xdev -size +2048 -ls |sort -r +6 
    echo "END WITHOUT email $(date) on hostname=$(ebs_host)" 
}
chk_status () 
{    
    : ~ 
    :     reports Warning if no df -kt
    set $(df -kt |grep 9[4-9]%)
    if [ -n "$1" ]
    then 
        warn_status "$1"
	exit 1
    else 
        ok_status
    fi
}

chk_status  $*  # the email addresses...

Comments and replies, where, in my reply

• "YES", means I've incorporated the comment in my code,
• "Q" means an open question, more exploration required,
• "MAINT", means a tentative "YES. most appropriate maintenance discussion.
• "TEST", means to consider testing hooks.

First, the refactorred script does not work. (1,2) Aesthetically, I had issues with the following: (3..n)

1. it uses ${x} where x is not defined.
   • YES. good insight, an oversight on my part, .e.g, see # 12
2. it uses a version of sort that my system does not have.
   • Q. on sort, I am aware of the -k flag on BSD systems is not an upward compatible feature of the earlier +m.n field-selectors for the sort. Where is the incompatibility on what system; I'd love to build the forward/backward compatibility switch-handler.
3. excessive use of echo – make yet another function with a here document.
   • MAINT. in this case, the very next thing to do with the script. The purpose in this case is to make the point, that now we have the functional factor, the real maintenance may begin. And this: eliminating successive echo's in behalf of a straight-forward here document completes the separation of the file-I/O from composing the message.
4. the pipeline with the sort options my system did not like appears twice – make it a function (so you can fix it in one place).
   • Q. sort, pipeline. sounds useful. specifically, the line of code?
5. do not pass things around as (a) global variable(s) unless they really are.
   • YES. move the variable to the one place it's used.p
6. put the global variables at the top whenever possible.
   • Q. "at the top". there are many "tops". e.g. a "main" function should have "global" initiation at the top. That main function may be defined last, first, or anywhere. the function library, on being sourced should announce it's entry point(s).
7. I prefer not to use other programs when shell has the functionality (I lost the grep).
   • MAINT. lost the grep. this is an example of "domain-specific" knowledge. e.g. the output of df and what it means, so keep the grep for the moment.
8. do not use exit unless it is essential.
   • MAINT. after the here file of #3, the next thing to consider. in a function-rich environment, the exit is unnecessary, if not a mistake. so, it deserves discussion with the author/user. Notice the chk_status call is really the "main" function here. If this file began with a sh-bang, then the exit is/may be appropriate. Since it doesn't, then entry/exit conditions are a worth a separate discussion.
9. comment on the reason for each function.
   • YES. I've a "comment" function. It makes the comment part of the semantics, invented to cover the declare -f idiom, the canonical function representation in the bash shell eats (deletes) the sharp-comments. In the intervening year, I've discovered it does preserve the colon-comment, but with some necessary care on the part of the user as it has evaluation properties. I'm updating.
10. global variable are very useful for stubbing out functionality, especially when testing (see ${TEE} & ${MAIL} ).
    • TEST. agree in principal, except the variables TEE and MAIL may be inadvertently left in the code. e.g. on the software assembly line, when something moves from unit test to functional, and before integration test, the delivered code can't be modified, implying the security hole of ${TEE} and ${MAIL} might not be repaired without the need for "retest". The better way to arrange the test harness is to insert hooks which may be removed w/o having to alter the delivered code. See Testing hooks
11. if-then-else-fi are sometimes better than && or || constructs (an interesting discussion lurks here).
    • YES. w.r.t #1, a good idea here. one of the things I'm changing in the intervening year. eg. && or || are "too clever by half"

To replace a built-in command or system utility to adequately test without a full system, or to capture well-known interfaces without generating the real behavior, a few testing hooks are possible. Here's a short list, realizing Diogenes Small's third observation: of the three possible explanations, it' usually one of the other seven. Here's the first three possibilities:

1. invoke a command using a variable (Environment variable) name,
2. insert a ../test/bin/ sufficiently early in the user's path,
3. create alias function with the same name and interface.

The first option is rejected out-of-hand, since it requires modifying tested code. The latter two are favored, since the tested code may be moved to the next testing station without change. The last one may be sub-divided even further, since the alias may be a function or a shell alias. The shell alias is rejected, even though likely the easiest to implement and control with user/environment features, it's application is likely easier to spoof than a separate executable file or function.

The alias function, little understood (an untested assertion on my part), is easily created. For example, to introduce a user-defined sort:

sort () { command sort ... ; }

where the ... is the interfacing command option(s). And more is possible. For example, I've been troubled for 75% (?!) of my shell programming life, with the interface to the comm command. My use of it, 99.44% of the time is comparing two lists of something or other: files, functions, list of ticker symbols, last, first names, … In this manner, comm performs Set Arithmetic. Consider the three operations: union, intersection, disjoint{a,b} between two lists in fileA and fileB:

sort -u fileA fileB       # the union
comm -12 fileA fileB      # the intersection
comm -23 fileA fileB      # disjointA.  A, not B

The two comm examples rely on an important assumption: the lists of fileA and fileB are sorted.

So, this alias function for comm fixes that oversight:

comm () 
{ 
    case $#."$1" in 
        2.-[123]*)
            report_notpipe && return 1;
            trace_call 2 minus N $*;
            sort -u - | command comm $1 - $(comm_tmp $2)
        ;;
        2.-)
            report_notpipe && return 2;
            trace_call 2 MINUS $*;
            sort -u | command comm - $(comm_tmp $2)
        ;;
        2.*)
            trace_call TWO $*;
            command comm $(comm_tmp $1) $(comm_tmp $2)
        ;;
        3.-[123]*)
            case "$2" in 
                -)
                    report_notpipe && return 3;
                    trace_call 3 MINUS $*;
                    sort -u | doit command comm $1 - $(comm_tmp $3)
                ;;
                *)
                    trace_call 3 STAR $*;
                    command comm $1 $(comm_tmp $2) $(comm_tmp $3)
                ;;
            esac
        ;;
        *)
            usage "comm [-mn] file | - file" OR comm Command Command
        ;;
    esac
}

It may be time to read a comm man(ual) page to appreciate the argument handling. By the way, the trace_call function logs its arguments, calling function, and that function's caller, while the report_notpipe function returns true if the input to the command or function is not on a pipe. The comm_tmp command returns the name of the file with sorted contents of its respective file argument.

And if you are really obsessive about these things, there is (at least) one bug in here, and two things worth noting:

1. None of the actual file argument names are wrapped in double quotes which might protect this against a space-afflicted file name, and
2. the comm_tmp function is necessary, since my limited understanding of the command built-in seems to butt into overuse of the <(... ), or shell-result-on-a-tmp-file feature.

I regard file names with spaces as a huge scam, perpetrated by Microsoft, as a means to confound command-line users into thinking point-and-click was productive. Not to mention, I learned my command line in the days of 14-character names, where spaces were considered useless, if not adding any information to the name. Big Hint: camelCase must have occurred before, if not in opposition to the space-name file.

I'm a big fan of the shell operators, || and &&, standing for the OR and AND results of the test operator. The program author is already using the … syntax for test, so let's go.

Beginning with the user's [existing script](https://dl.dropboxusercontent.com/u/66403621/shellfunctions/factor/txt/ebsdfwarn_001.txt)

Here is the [first modifications](https://dl.dropboxusercontent.com/u/66403621/shellfunctions/factor/txt/ebsdfwarn_002.txt)

Now introduce SYSID as concatenation of two variables which will later be eliminated.

after consolidating [two variables into one,](https://dl.dropboxusercontent.com/u/66403621/shellfunctions/factor/txt/ebsdfwarn_003.txt)

the next change uses dollar paren … paren "$( … )" logic, to isolate what had been a variable, into an expression:

[[ -n $(df -kt |grep 9[4-9]%) ]] || {

This replaces the 'x' variable with internal evaluation. (a too-busy evaluation is an excellent place for a function)

this is a set-up prior to eliminating it

the syswarn function delivers on a promise

begin removing SYSID variable by using it as a function argument

eliminating MAILLIST, and replacing with literal arguments for a moment shows where the ultimate high-level command arguments, i.e. the destination email addresses will be supplied.

Add ok\_status and warn\_status functions taking same SYSID as argument.

And eliminate HSTNM, OSLVL variables which collapse into SYSID.

Use warning file explicitly in the one place it is required, anticipating ENVIRONMENT variable at end.

by passing the argument to the new function chk_status, issues either warning or OK.

This allows chk_status to pass SYSID ema1 ema2 … emaN arguments

Then, maybe the … tee file … may be altogether eliminated.