Code bloat: Why should meta_trap_err() display error about itself?

[Explorer09]

I can't grab the idea behind this design:

meta_trap_err(){
    if [ ${#} -ne 4 ]; then
        printf 'ERROR: %s: Wrong function argument quantity!\n' "${FUNCNAME[0]}" 1>&2
        return "${COMMON_RESULT_FAILURE}"
    fi
    local -ir line_error_location=${1}; shift # The line number that triggers the error
    local -r failing_command="${1}"; shift # The failing command
    local -ir failing_command_return_status=${1}; shift # The failing command's return value
    local -r failing_function="${1}"
    meta_trap_err_print_debugging_info "${line_error_location}" "${failing_command}" "${failing_command_return_status}" "${failing_function}"
    return "${COMMON_RESULT_SUCCESS}"
}; declare -fr meta_trap_err

And this:

meta_trap_err_print_debugging_info(){
    if [ ${#} -ne 4 ]; then
        printf 'ERROR: %s: Wrong function argument quantity!\n' "${FUNCNAME[0]}" 1>&2
        return "${COMMON_RESULT_FAILURE}"
    fi
...

Functions intending to display errors need to display error about themselves? (Funny error message picture: Dialog box saying "An error occurred while displaying previous error.") Enough said.

[brlin-tw]

This kind of routines should be replaced by an unimplemented call of:

## Utility function to check if function parameters quantity is legal
## NOTE: non-static function parameter quantity(e.g. either 2 or 3) is not supported
util_check_function_parameters_quantity(){
    if [ "${#}" -ne 2 ]; then
        printf_with_color\
            red\
            '%s: FATAL: Function requires %u parameters, but %u is given\n'\
            "${FUNCNAME[0]}"\
            2\
            "${#}"
        exit 1
    fi

    # The expected given quantity
    local -i expected_parameter_quantity="${1}"; shift
    # The actual given parameter quantity, simply pass "${#}" in the caller will do
    local -i given_parameter_quantity="${1}"

    if [ "${given_parameter_quantity}" -ne "${expected_parameter_quantity}" ]; then
        printf_with_color\
            red\
            '%s: FATAL: Function requires %u parameters, but %u is given\n'\
            "${FUNCNAME[1]}"\
            "${expected_parameter_quantity}"\
            "${given_parameter_quantity}"\
            1>&2
        exit 1
    fi
    return 0
}; declare -fr util_check_function_parameters_quantity

An example of its usage:

cleanup_mountpoints(){
    util_check_function_parameters_quantity 3 "${#}"
    local -r source_fs_mountpoint="${1}"; shift
    local -r target_fs_mountpoint="${1}"; shift
    local -ir only_for_gui="${1}"

...
}

The error is rather a FATAL one as it indicates that a function is called in a way that it isn't designed at all, and should be fixed immediately when found cause this is a straight-forward design problem. You may analogy it as an assertion.

Hope that answers your question.

[Explorer09]

@Lin-Buo-Ren My argument was about unnecessary bloat. It is quite unlikely that your util_check_function_parameters_quantity() or meta_trap_err_print_debugging_info() or whatever routine will be called by outside (user code), and so why bother with the complication?

With the example code you gave for util_check_function_parameters_quantity(), didn't you notice that you checked the number of arguments of the function itself and then checked the number from whatever calls it, and that the error message is duplicated?

util_check_function_parameters_quantity() {
  # Check parameters' quantity of itself... Oops. Infinite recursion!!
  util_check_function_parameters_quantity 2 ${#}

  local -i expected_parameter_quantity="${1}"; shift
  local -i given_parameter_quantity="${1}"
}

Technically you bootstrapped it to avoid the problem of recursion. But here, maybe the chicken-and-egg are all totally redundant. If I write the script, then [ "${#}" -eq "$quantity" ] or [ "${#}" -ne "$quantity" ] will simply do the job and no need for the stupid wrapper.

[brlin-tw]

If I write the script, then [ "${#}" -eq "$quantity" ] or [ "${#}" -ne "$quantity" ] will simply do the job and no need for the stupid wrapper.

...which complicates the code by inserting obscured tests at each heading of a function, while exploiting util_check_function_parameters_quantity you only need to consider how many function parameters you accept, and pass the argument # that is actually passed by the callee for the utility function to check for you.

And as you've stated I avoid the recursion issue by only self-implementing the same routine twice, which is far less work than doing it in ${all_of_my_functions[*]} while also improves code readability

[Explorer09]

@Lin-Buo-Ren Then what if I make a function that accepts a variable number of arguments? And to check whether there are a minimum number (instead of exact number) of arguments? I think such cases would be many, for example those which accept a list of files to process (ls, cp, mv, tar, chown, etc).

[brlin-tw]

Then what if I make a function that accepts a variable number of arguments?

We simply don't support it, as stated in the NOTE of the implementation of util_check_function_parameters_quantity.

I think such cases would be many, for example those which accept a list of files to process

Here is an example in practice that utilizes util_check_function_parameters_quantity: https://github.com/slacka/WoeUSB/blob/master/src/woeusb, it has called the utility function 39 times while the total function # is 45, which is over 85% in quantity.

[Explorer09]

@Lin-Buo-Ren It might be just me, but I really hate when people replicate a functionality that is provided already in the shell environment, let alone with obscure and long function names.

If I need an assert, I'll write this way then:

assert () {
"$@" || { s=$?; printf '%s: assertion error: %s\n' "$0" "$*" >&2; exit $s; } 
}

my_func () {
assert [ "$#" -eq 2 ]
}