Skip to main content
added 176 characters in body
Source Link
ilkkachu
  • 147.9k
  • 16
  • 268
  • 441

Well,

if [ "$1" == "" ] || [ $# -gt 1 ]; then
        echo "Parameter 1 is empty"
if [ "$1" == "" ] || [ $# -gt 1 ]; then
        echo "Parameter 1 is empty"

First, use = instead of ==. The former is standard, the latter a bashism (though I think it's from ksh, too). Second, the logic here isn't right: if $# is greater than one, then parameter 1 probably isn't empty (it might be set to the empty string, though). Perhaps you meant "$#" -lt 1, though that would also imply that "$1" = "". It should be enough to test [ "$1" = "" ], or [ "$#" -lt 1 ].

elif [! "${#timestamp}" -gt 10 ]; then
elif [! "${#timestamp}" -gt 10 ]; then

Here, the shell would try to run a command called [! (literally). You need a space in between, so [ ! "${#timestamp}" -gt 10 ]. But that's the same as [ "${#timestamp}" -le 10 ], which would also catch strings of exactly 10 characters, like 2018-08-14. 

So maybe you want [ "${#timestamp}" !=-ne 10 ]. (!= instead of -ne would also work, even though it's a string comparison.)

if ...
    exit 0

It's also customary to return with a non-zero exit code in case of an error, so use exit 1 in the error branches.

 

You could also use case or [[ .. ]] to pattern match the argument against the expected format, e.g.:

case "$1" in
    "")
        echo date is empty
        exit 1;;
    [0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9])
        echo date is ok;;
    *)
        echo "date is _not_ ok"
        exit 1;;
esac

That would also refusereject arguments like abcdefghij, even though it's 10 characters long.

Well,

if [ "$1" == "" ] || [ $# -gt 1 ]; then
        echo "Parameter 1 is empty"

First, use = instead of ==. The former is standard, the latter a bashism (though I think it's from ksh, too). Second, the logic here isn't right: if $# is greater than one, then parameter 1 probably isn't empty (it might be set to the empty string, though). Perhaps you meant "$#" -lt 1, though that would also imply that "$1" = "". It should be enough to test [ "$1" = "" ], or [ "$#" -lt 1 ].

elif [! "${#timestamp}" -gt 10 ]; then

Here, the shell would try to run a command called [! (literally). You need a space in between, so [ ! "${#timestamp}" -gt 10 ]. But that's the same as [ "${#timestamp}" -le 10 ], which would also catch strings of exactly 10 characters, like 2018-08-14. So maybe you want [ "${#timestamp}" != 10 ].

It's also customary to return with a non-zero exit code in case of an error, so use exit 1 in the error branches.

You could also use case or [[ .. ]] to pattern match, e.g.:

case "$1" in
    "")
        echo date is empty
        exit 1;;
    [0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9])
        echo date is ok;;
    *)
        echo "date is _not_ ok"
        exit 1;;
esac

That would also refuse arguments like abcdefghij, even though it's 10 characters long.

Well,

if [ "$1" == "" ] || [ $# -gt 1 ]; then
        echo "Parameter 1 is empty"

First, use = instead of ==. The former is standard, the latter a bashism (though I think it's from ksh, too). Second, the logic here isn't right: if $# is greater than one, then parameter 1 probably isn't empty (it might be set to the empty string, though). Perhaps you meant "$#" -lt 1, though that would also imply that "$1" = "". It should be enough to test [ "$1" = "" ], or [ "$#" -lt 1 ].

elif [! "${#timestamp}" -gt 10 ]; then

Here, the shell would try to run a command called [! (literally). You need a space in between, so [ ! "${#timestamp}" -gt 10 ]. But that's the same as [ "${#timestamp}" -le 10 ], which would also catch strings of exactly 10 characters, like 2018-08-14. 

So maybe you want [ "${#timestamp}" -ne 10 ]. (!= instead of -ne would also work, even though it's a string comparison.)

if ...
    exit 0

It's customary to return with a non-zero exit code in case of an error, so use exit 1 in the error branches.

 

You could also use case or [[ .. ]] to pattern match the argument against the expected format, e.g.:

case "$1" in
    "")
        echo date is empty
        exit 1;;
    [0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9])
        echo date is ok;;
    *)
        echo "date is _not_ ok"
        exit 1;;
esac

That would also reject arguments like abcdefghij, even though it's 10 characters long.

Source Link
ilkkachu
  • 147.9k
  • 16
  • 268
  • 441

Well,

if [ "$1" == "" ] || [ $# -gt 1 ]; then
        echo "Parameter 1 is empty"

First, use = instead of ==. The former is standard, the latter a bashism (though I think it's from ksh, too). Second, the logic here isn't right: if $# is greater than one, then parameter 1 probably isn't empty (it might be set to the empty string, though). Perhaps you meant "$#" -lt 1, though that would also imply that "$1" = "". It should be enough to test [ "$1" = "" ], or [ "$#" -lt 1 ].

elif [! "${#timestamp}" -gt 10 ]; then

Here, the shell would try to run a command called [! (literally). You need a space in between, so [ ! "${#timestamp}" -gt 10 ]. But that's the same as [ "${#timestamp}" -le 10 ], which would also catch strings of exactly 10 characters, like 2018-08-14. So maybe you want [ "${#timestamp}" != 10 ].

It's also customary to return with a non-zero exit code in case of an error, so use exit 1 in the error branches.

You could also use case or [[ .. ]] to pattern match, e.g.:

case "$1" in
    "")
        echo date is empty
        exit 1;;
    [0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9])
        echo date is ok;;
    *)
        echo "date is _not_ ok"
        exit 1;;
esac

That would also refuse arguments like abcdefghij, even though it's 10 characters long.