2
if [[ ${1: -1}=='C' ]]
then
  celsius=$(echo $1 | sed 's/C//g')
  echo "`expr $celsius + 273`K"
else
  kelvin=$(echo $1 | sed 's/K//g')
  echo "`expr $kelvin - 273`C"
fi

please explain me what's wrong

./script 55C

328K

./script 122K

expr: not a decimal number: '122K'

K

Estet
  • 147
  • 7
  • 2
    No need to use `expr` for arithmetic in shell. Just use, e.g., `$(($celsius + 273))`. – chepner Jan 06 '23 at 19:59
  • [Shellcheck](https://www.shellcheck.net/) identifies several problems with the code. Follow the links in the [Shellcheck](https://www.shellcheck.net/) report for information about how to fix the problems. – pjh Jan 06 '23 at 22:21

3 Answers3

3

Like this to avoid if forest ('if' means a kind of tree in French):

#!/bin/bash

case $1 in
    *[cC])
        echo "$((${1%[a-zA-Z]} + 273))K"
    ;;
    *[kK])
        echo "$((${1%[a-zA-Z]} - 273))C"
    ;;
    *)
        echo "error arg [$1]" >&2
        exit 1
    ;;
esac

expr is a program used in ancient shell code to do math.
In Posix shells like bash, use $(( expression )).
In bash, ksh88+, mksh/pdksh, or zsh, you can also use (( expression ))

A float precision capable version, using bc and the real float 273.16:

#!/bin/bash

case $1 in
    *[cC])
        echo "$(bc <<< "scale=2; ${1%[a-zA-Z]} + 273.16")K"
    ;;
    *[kK])
        echo "$(bc <<< "scale=2; ${1%[a-zA-Z]} - 273.16")C"
    ;;
    *)
        echo "error arg [$1]" >&2
        exit 1
    ;;
esac
Gilles Quénot
  • 173,512
  • 41
  • 224
  • 223
1

Missing spaces. The operator didn't work properly, so it always goes to the first if expression.

-if [[ ${1: -1}=='C' ]]              
+if [[ ${1: -1} == 'C' ]]
 then
   celsius=$(echo $1 | sed 's/C//g')
   echo "`expr $celsius + 273`K"
 else
   kelvin=$(echo $1 | sed 's/K//g')
   echo "`expr $kelvin - 273`C"
 fi
Rein Avila
  • 375
  • 1
  • 6
  • 1
    Yes. The `[[` command does different things depending on how many arguments it gets. With the missing spaces, it gets just one argument (not counting the ending `]]`), and the test in that case is "is this an empty string?" – glenn jackman Jan 06 '23 at 19:56
  • Quite inefficient compared to `case $1 in *C) echo "$(( ${1%C} + 273 ))K";; *K) echo "$(( ${1%K} - 273 ))C";; esac` – Charles Duffy Jan 06 '23 at 20:03
  • `expr` is a holdout from the 1970s; ever since the 1992 publication of POSIX.2, the shell has been mandated to have built-in math support that's far faster (since using it doesn't require starting a separate executable in a subprocess). – Charles Duffy Jan 06 '23 at 20:04
  • Backtick-based command substitution also has its own problems -- it doesn't nest well, and code using backslashes or further backticks needs to be modified when they're in use. See https://wiki.bash-hackers.org/scripting/obsolete for discussion of `expr` and backticks both. – Charles Duffy Jan 06 '23 at 20:06
  • Sorry everyone. I thought the post is only about "please explain me what's wrong" - what's the bug. Didn't realize I should also rewrite the whole code. – Rein Avila Jan 11 '23 at 12:28
1

You can also do something like this

#! /bin/bash

u="${1: -1}"
case "$u" in
    K)
        t="${1/K/-}"
        u='C'
        ;;

    C)
        t="${1/C/+}"
        u='K'
        ;;

    *)
        echo "error" >&2
        exit 1
        ;;
esac

echo "$(($t 273))$u"
Diego Torres Milano
  • 65,697
  • 9
  • 111
  • 134