How to correctly return values in shell script function for edge cases?

Question

getAnimalFolder() {
    local animal=""
    if [[ ${ANIMAL} == "lion" ]]; then
        animal = "./animals/lion/"
    elif [[ ${ANIMAL} == "tiger" ]]; then
        animal = "./animals/tiger/"
    elif [[ ${ANIMAL} == "cheetah" ]]; then
        animal = "./animals/cheetah/"
    else
        echo "inavalid animal"
        exit 1`enter code here`
    fi
    echo $animal
}

result=$(getAnimalFolder)
cd ../result/age/

If the animal is not lion, tiger or cheetah, the function returns invalid animal and hence gives an error 'No such file or directory', instead I need to do an exit with code = 1. Hence I went for the second option -

if [[ ${ANIMAL} != "lion" && ${ANIMAL} != "tiger" && ${ANIMAL} != "cheetah" ]]; then
    echo "Invalid animal"
    exit 1
fi
getAnimalFolder() {
        local animal=""
        if [[ ${ANIMAL} == "lion" ]]; then
            animal = "./animals/lion/"
        elif [[ ${ANIMAL} == "tiger" ]]; then
            animal = "./animals/tiger/"
        elif [[ ${ANIMAL} == "cheetah" ]]; then
            animal = "./animals/cheetah/"
        fi
        echo $animal
}
result=$(getAnimalFolder)
cd ../result/age/

This looks like a fix to my problem but if in the future more animals are added, then I need to remember to make changes in 2 places for every new animal added. So is there a better way to do this?

Answer 1

There are a number of problems here; #1 and #3 are the ones that directly address your question.

1. When a function/command/whatever may need to print both regular output (e.g. the path to an animal directory) and error/status output (e.g. "inavalid animal"), it should send the regular output to standard output (aka stdout aka FD #1, the default), and error/status output to standard error (aka stderr aka FD #2), like this:

   echo "Invalid animal" >&2    # This is sent to stderr
   

2. Generally, functions should return rather than exiting. If a function does exit, it exits the entire shell, but in this case the function is running in a subshell due to $( ), so it only exits that. Using return avoids this inconsistency.

3. When a function/command/whatever may fail, you should check its exit status; there are a number of ways to do this:

   if result=$(getAnimalFolder); then
       : # command succeeded!
   else
       echo "OMG it failed!" >&2
       exit 1
   fi
   

   or

   result=$(getAnimalFolder)
   if [ $? -ne 0 ]; then    # $? is the status of the last command
       echo "OMG it failed!" >&2
       exit 1
   fi
   

   or

   result=$(getAnimalFolder) || {
       echo "OMG it failed!" >&2
       exit 1
   }
   

   I use the last form a lot, since there are a lot of steps in a script might fail, and having a simple & compact way to include the failure handing code makes the overall script more readable.

4. In general, functions should take their input as arguments rather than via global variables. So in the function you'd refer to $1 instead of $ANIMAL, and you'd run the function with something like:

   result=$(getAnimalFolder "$ANIMAL")
   

There are also a number of basic syntax errors and bad scripting practices in the script: don't put spaces around the equal sign in assignments; do put double-quotes around variable references; don't use all-caps variable names (to avoid conflicts with the many all-caps variables that have special meanings); do check for errors on cd commands (if they fail, the rest of the script will run in the wrong place); and when comparing a single variable against a bunch of values, use case instead of a bunch of if elseif etc.

shellcheck.net is good at recognizing many of these common mistakes. Strongly recommended.

Here's what I get with all fixes in place:

#!/bin/bash

getAnimalFolder() {
    local animalname=$1
    local animaldir=""
    case "$animalname" in
        lion ) animaldir="./animals/lion/" ;;

        tiger ) animaldir="./animals/tiger/" ;;

        cheetah ) animaldir="./animals/cheetah/" ;;

        * )
            echo "Invalid animal: $animalname" >&2
            return 1 ;;
    esac
    echo "$animaldir"
}

read -p "Give me an animal: " animalname
result=$(getAnimalFolder "$animalname") || {
    exit 1 # Appropriate error message has already been printed
}
cd "../$result/age/" || {
    echo "Error changing directory to ../$result/age/ -- aborting" >&2
    exit 1
}

Answer 2

Put the animals in an array:

#!/bin/bash
animals=(lion tiger cheetah)

getAnimalFolder() {
   local i
   for i in "${animals[@]}"; do
      if [ "$i" == "${1}" ] ; then
        animaldir="./animals/${1}"
        return 0
      fi
   done
   exit 1
}

read -rp "Give me an animal: " animalname
getAnimalFolder "${animalname}"

echo "Animaldir=${animaldir}"

EDIT: I did not use the construction result=$(getAnimalFolder), assuming the OP wants to use the new path once. When needed, the function can be changed into

    echo "./animals/${1}"

When the function is called with result=$(getAnimalFolder), OP needs to look at the line

cd ../result/age/

Is resulta fixed path or does he want to use the path from the function: cd ../${result}/age/