CODEWITHSUNDEEP
bash
Turnipdabeets
Turnipdabeets

Reputation: 6015

bash function return in if statement not working

I'm trying to refactor this code:

if [ $(($1 % 4)) -eq 0 ] && [ $(($1 % 100)) -ne 0 ] || [ $(($1 % 400)) -eq 0 ] ; then
    echo $T
else
    echo $F
fi

into something like this:

if divisibleBy4 && notDivisibleBy100 || divisibleBy400; then
    echo $T
else
    echo $F
fi

note that 

T="true"
F="false"

divisibleBy4 function looks like:

divisibleBy4() {
    return  [ $(($1 % 4)) -eq 0 ]
}

But I've tried several iterations including what I thought would definitely work. 

divisibleBy4() {
    if [ $(($1 % 4)) -eq 0  ]; then
    return 1
    else return 0
    fi
}

Any idea how to properly fix the syntax so I can refactor these into functions and use them in my if statement?

When testing I'm seeing the error

syntax error: operand expected (error token is "% 4")

Another thing I tried is, but still doesn't seem to work:

INPUT=$1

divisibleBy4() {
    if [ $(($INPUT % 4)) -eq 0  ]; then
         return 1
    else return 0
    fi
}

notDivisibleBy100() {
    if [ $(($INPUT % 100)) -ne 0]; then
         return 1
    else return 0
    fi
}

divisibleBy400() {
    if [ $(($INPUT % 400)) -eq 0  ]; then
         return 1
    else return 0
    fi
}


if divisibleBy4 && notDivisibleBy100 || divisibleBy400; then
    echo $T
else
    echo $F
fi

or

INPUT=$1

divisibleBy4() {
    return $((!($INPUT %4)))
}
notDivisibleBy100() {
    return $(($INPUT %100))
}
divisibleBy400() {
    return $((!($INPUT %400)))
}

(( divisibleBy4 && notDivisibleBy100 || divisibleBy400 )) && echo "true" || echo "false"

Upvotes: 0

Views: 2548

Answers (3)

tripleee
tripleee

Reputation: 189936

Every command sets a result code. If you want to force each calculation to happen in a separate function, you can say

divisibleBy4() {
    $((!("$1" % 4)))
}
notDivisibleBy100() {
     $(("$1" %100))
}
divisibleBy400() {
     $((!("$1" %400)))
}

(divisibleBy4 "$1" &&
 notDivisibleBy100 "$1" ||
 divisibleBy400 "$1") &&
echo "true" || echo "false"

Breaking up your logic to functions on the subatomic level is not really helping legibility and maintainability, though. Perhaps if you want to make each part reasonably self-domumenting, use comments.

is_leap () {
    # Divisible by 4?
    (("$1" % 4 == 0)) || return
    # Not divisible by 100?
    (("$1" % 100 > 0)) && return
    # Divisible by 400?
    (("$1" % 400 == 0))
}

... Though the comments seem rather superfluous here.

Upvotes: 0

Gordon Davisson
Gordon Davisson

Reputation: 126108

The simplest, directest answer is to just create functions that consist only of the tests themselves:

INPUT=$1

divisibleBy4() {
    [ $(($INPUT % 4)) -eq 0  ]
}

notDivisibleBy100() {
    [ $(($INPUT % 100)) -ne 0 ]
}

divisibleBy400() {
    [ $(($INPUT % 400)) -eq 0  ]
}

The reason this works is that a function without a return will implicitly return the status of the last command in the function; in these cases, that's the test command (note: [ is a command, even though it doesn't look like one), so the functions just return the result of the test directly.

I'd make at least one change to these, though: they all test the value of the shell variable INPUT; it's much better practice to actually pass the data that functions operate on as parameters. Thus, it'd be better to do something like this:

divisibleBy4() {
    [ $(($1 % 4)) -eq 0  ]
}

if divisibleBy4 "$1" ...

Rather than this:

divisibleBy4() {
    [ $(($INPUT % 4)) -eq 0  ]
}

INPUT=$1
if divisibleBy4 ...

Note that you can also bundle up the whole leap year check the same way:

isLeapYear() {
    [ $(($1 % 4)) -eq 0 ] && [ $(($1 % 100)) -ne 0 ] || [ $(($1 % 400)) -eq 0 ]
}

if isLeapYear "$1"; then

Or use the simpler form @Wiimm suggested:

isLeapYear() {
    (( !($1%4) && $1%100 || !($1%400) ))
}

Also, for the shell variables you do use, lower- or mixed-case is preferred, to avoid accidental conflicts with the many all-caps variable names that have special meanings or functions.

Upvotes: 1

Wiimm
Wiimm

Reputation: 3615

You want to detect a leap year! A complete other solution using math mode directly:

a="$1"; (( !(a%4) && a%100 || !(a%400) )) && echo true || echo false

or as if-then-else

a="$1";
if (( !(a%4) && a%100 || !(a%400) )); then
    echo true
else
    echo false

Upvotes: 3

Related Questions