I am new to bash scripting and I wrote a script using a function to calculate the factorial of a given number. The function works well for zero and positive numbers but not it does not give the expected output for the negative values.

Here is my script:

#!/bin/bash

# factorial program using a function with while loop

calculate_factorial () {
        result=1
        current=1



        if(( $1 < 0 )); then
                echo "The number cannot be negative"

        elif(( $1 == 0 )); then
                 echo "1"
        else
                while(($current <= $1)); do
                        result=$(( result*current ))
                        current=$(( current 1 ))
                done
        #print the result
        return $result
        fi

}

calculate_factorial $1

echo $result

The output for -8:

The number cannot be negative
1

It was supposed to only output The number cannot be negative but I don't know where 1 came from in the second line of the output.

I would appreciate it if you could spot my mistake if I have any or explain the reason.

CodePudding user response：

The short answer it that it's because you set result=1 at the beginning of the function (note that since result isn't declared as local, it's a global variable), and at the end of the main script you echo $result. result is still set to "1", so that's what it prints.

The longer answer is that you're misunderstanding how to return a value from a function. In general, functions can produce three kinds of results:

1. To return data (in this case, the factorial value), you should print it to standard output (aka stdout, which is the default output target). You can use echo or any other command that produces output. Do not use the return command for that (see below). You do this correctly in the (( $1 == 0 )) case.

   If you need to capture the output when using the function, you can use value=$(functname ...args...), but in this case it looks like you just want to print it anyway, so you don't need to capture the output, just let it go straight to the terminal.

2. To return an error or status message (like "The number cannot be negative"), print it to standard error (aka stderr) rather than standard output. You can redirect a command's output to standard error with >&2.

3. To return a success/failure status, use the return command (0=success, nonzero=failure). This is all you should ever return in the return command (and similarly, the exit value from a script). If you want, you can use different nonzero values to indicate different problems, but most things just use 1 for all errors.

   To check the return status of a function, either embed it in something like an if statement, or check $? immediately after calling the function (it holds the status of the most recent command, so if you run any other command, that'll replace it).

Also, it's generally good scripting hygiene to double-quote variable and parameter references (e.g. "$1" instead of just $1) to avoid weird parsing. There are a few exceptions, like inside a (( )) expression. Also, inside (( )) or other arithmetic contexts, you don't need to use $ to get the value of variables. shellcheck.net it good at pointing out things like this. BTW, in shell syntax, spaces are extremely important delimiters. Using if(( (without a space between) happens to work, but it's a much better to get in the habit of separating elements like if (( (except, of course, in cases where they're required not to be separated, like var=value).

So, here's a corrected version of your function:

#!/bin/bash

# factorial program using a function with while loop

calculate_factorial () {
        result=1
        current=1

        if (( $1 < 0 )); then
                # Here, we print an error message to stderr
                echo "The number cannot be negative" >&2
                # and then return an error status
                return 1

        elif (( $1 == 0 )); then
                # Here, we print the result to stdout
                echo "1"
                # and then return a success status
                return 0

        else
                while (( current <= $1 )); do
                        result=$(( result*current ))
                        current=$(( current 1 ))
                done
                #print the result
                echo "$result"
                # and then return a success status
                return 0
        fi

}

calculate_factorial "$1"

CodePudding user response：

Change echo "The number cannot be negative" to result="The number cannot be negative