4

I have a file that I have some functions to be used by a script.
So in my script I do the following:

source my_functions.sh   

VALUE_A=$(get_proper_value "X")  
[[ -z "$VALUE_A" ]] && echo "Missing value" && exit 1  

THE_URL=$(get_url_of_service "SERVICE_NAME")  
[[ -z "$THE_URL" ]] && echo "Missing URL" && exit 1    

CUSTOMER_ID=$(generate_customer_id "Z")  
[[ -z "$CUSTOMER_ID" ]] && echo "Missing customer id" && exit 1      

etc  

I have 4-5 more such declarations at the top of my script before actually doing any processing.
This seems to clutter the script.

Is there a better approach for this? I was thinking of declaring the variables inside my_functions.sh but I was wondering if that could make it worse as it won’t be clear where these variables are coming from

  • @Kusalananda:I fixed that – Jim Dec 5 '17 at 10:29
  • 2
    Just an idea: use the ${parameter:?word} parameter expansion for outputting the configuration values in the routines that generate the data to make them report the error if the parameter is unset or null. See the Bash manual. – Kusalananda Dec 5 '17 at 10:30
  • @Kusalananda: I didn't understand what you mean – Jim Dec 6 '17 at 8:47
  • Basically make it a fatal error in the routine that creates the value if the value is null or empty, before returning only valid values to the code in your example. – Kusalananda Dec 6 '17 at 8:51
1

No, what you are doing is a coding pattern called "guard clauses" which is a good thing to do. Handling these trivial error cases later in the code may make it more complicated than necessary.

  • Ok but is there a way to somehow "hide" this part from the main script functionality? – Jim Dec 5 '17 at 11:13
  • 1
    if you are just testing for value existence (and won't test for value range etc...) then @Kusalananda's suggestion is the way to go. But at that point clutter is in the eye of the beholder. If the purpose of that section is obvious, it is easy to skip when reading the source. – xenoid Dec 5 '17 at 12:51
1

Well, of course there are ways to make the tests more brief, e.g. put the repeating part in a function

notempty() { for val; do [ -z "$val" ] && return 1; done; return 0; }

VALUE_A=$(...)
THE_URL=...
CUSTOMER_ID=...  
notempty "$VALUE_A" "$THE_URL" "$CUSTOMER_ID" || { echo required variable not set >&2; exit 1; }

But that doesn't let you give a custom error message for each variable. How about misusing indirect references to at least print the names of the variables?

notempty_ind() { 
    for var; do 
        if [ -z "${!var}" ]; then 
            echo "variable $var not set!" >&2; 
            return 1;
        fi;
    done; 
    return 0; 
}
...  
notempty_ind VALUE_A THE_URL CUSTOMER_ID || exit 1

Or just replacing the ifs with the ${parameter:?message} expansion @ Kusalananda mentioned. It exits the shell and prints an error message if the parameter is unset or empty. (See e.g. the Bash manual)

CUSTOMER_ID=...
: "${CUSTOMER_ID:?customer id not set!}" "${THE_URL:?}"

The error message from Bash includes the variable name, so the custom message isn't really needed, if the name is clear enough.

  • The parameter expansion won't help as I want the script to terminate if there is no value. I don't want to assign any default value.Also in the notempty_ind shouldn't that return 0; when var is not set? – Jim Dec 6 '17 at 14:35
  • @Jim, terminating is exactly what the ${param:?message} expansion does if param is unset or empty. It's ${par:-val} that uses a default value and ${par:=val} that assigns and uses the default value. And hmm, notempty returns 0 i.e. ok if the arguments are all nonempty, and the || exit exits if the function fails (i.e. returned 1), so doesn't that work right, then? Of course you could invert the sense of the return value, or have the function exit directly. – ilkkachu Dec 6 '17 at 15:38
1

There's a couple of things that I can think of that would make the main script less cluttered.

  1. Make your functions return a non-zero exit code if the value could not be determined, using return 1. Alternatively, make them actually terminate the script using exit.

    Making them return a non-zero exit status would work well together with using a name reference variable:

    set -e
    
    generate_customer_id CUSTOMER_ID "Z"
    

    Here we assume that generate_customer_id takes the name of the variable to set as the first argument (it would use typeset -n var="$1" in its code and set var=...something... before returning). If it fails to get the customer ID for its second argument, it would return 1. The main script would then exit due to the set -e (this makes the shell exit as soon as a command returns non-zero).

  2. In addition to the above, or possibly instead of the above, use set -u. This would make the script exit as soon as you try to expand the value of a variable that is not currently set.

0

Following up on @xenoid's and @ilkkachu's answers, with a recent version of bash you can write this, using a nameref variable in the function

guard_var() {
    local -n var=$1    
    local value=$2
    local desc=$3

    if [[ $value ]]; then
        var=$value
    else
        echo "Missing $desc"
        exit 1
    fi
}

guard_var  valueA      "$(get_proper_value "X")"               "value"
guard_var  theUrl      "$(get_url_of_service "SERVICE_NAME")"  "URL"
guard_var  customerId  "$(generate_customer_id "Z")"           "customer id"

and the guard_var function can be placed in your my_functions.sh file to get it out of the script.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.