20

How may I shorten this shell script?

CODE="A"

if test "$CODE" = "A"
then
 PN="com.tencent.ig"
elif test "$CODE" = "a"
 then
 PN="com.tencent.ig"
elif test "$CODE" = "B"
 then
 PN="com.vng.pubgmobile"
elif test "$CODE" = "b"
 then
 PN="com.vng.pubgmobile"
elif test "$CODE" = "C"
 then
 PN="com.pubg.krmobile"
elif test "$CODE" = "c"
 then
 PN="com.pubg.krmobile"
elif test "$CODE" = "D"
 then
 PN="com.rekoo.pubgm"
elif test "$CODE" = "d"
 then
 PN="com.rekoo.pubgm"
else
 echo -e "\a\t ERROR!"
 echo -e "\a\t CODE KOSONG"
 echo -e "\a\t MELAKUKAN EXIT OTOMATIS"
 exit
fi
  • 2
    I suppose this is bash code? Or do you have any other shell in mind? – Freddy Jul 24 '19 at 6:20
  • 3
    FYI in the future, i would recommend replacing personal information like URLs and other things with something generic like "com.hello.world". – Trevor Boyd Smith Jul 24 '19 at 19:58
  • 1
    @IISomeOneII You should be asking CodeGolf.SE instead :P – mackycheese21 Jul 25 '19 at 21:42
  • 3
    @Trevor, I'd recommend example.org, example.net etc, as these domains are specifically reserved for this purpose in RFC 2606 and will never be used for real entities. – Toby Speight Jul 26 '19 at 8:46
  • 2
    @TrevorBoydSmith Seconding Toby's recommendation of com.example etc., since "hello.com" is owned by Google. – David Conrad Jul 26 '19 at 16:52
62

Use a case statement (portable, works in any sh-like shell):

case "$CODE" in
    [aA] ) PN="com.tencent.ig" ;;
    [bB] ) PN="com.vng.pubgmobile" ;;
    [cC] ) PN="com.pubg.krmobile" ;;
    [dD] ) PN="com.rekoo.pubgm" ;;
    * ) printf '\a\t%s\n' 'ERROR!' 'CODE KOSONG' 'MELAKUKAN EXIT OTOMATIS' >&2
        exit 1 ;;
esac

I'd also recommend changing your variable names from all capital letters (like CODE) to something lower- or mixed-case (like code or Code). There are many all-caps names that have special meanings, and re-using one of them by accident can cause trouble.

Other notes: The standard convention is to send error messages to "standard error" rather than "standard output"; the >&2 redirect does this. Also, if a script (or program) fails, it's best to exit with a nonzero status (exit 1), so any calling context can tell what went wrong. It's also possible to use different statuses to indicate different problems (see the "EXIT CODES" section of the curl man page for a good example). (Credit to Stéphane Chazelas and Monty Harder for suggestions here.)

I recommend printf instead of echo -e (and echo -n), because it's more portable between OSes, versions, settings, etc. I once had a bunch of my scripts break because an OS update included a version of bash compiled with different options, which changed how echo behaved.

The double-quotes around $CODE aren't really needed here. The string in a case is one of the few contexts where it's safe to leave them off. However, I prefer to double-quote variable references unless there's a specific reason not to, because it's hard to keep track of where it's safe and where it isn't, so it's safer to just habitually double-quote them.

|improve this answer|||||
  • 5
    @IISomeOneII That will count as * (and print the error) -- the pattern [aA] matches either "a" or "A", but not both at once. – Gordon Davisson Jul 24 '19 at 6:38
  • 6
    This is exactly the right way to do it, right down to the wildcard at the end redirecting its output to stderr and generating a non-zero exit value. The only thing that might need to change is that exit value, as there may be more than one error to return. In a larger script, there might be a section (perhaps sourced from another file) that defines exit vales readonly Exit_BadCode=1 so that it can say exit $Exit_BadCode instead. – Monty Harder Jul 24 '19 at 19:23
  • 2
    If going with a recent bash, then use case "${CODE,}" in, so that each of the conditionals becomes simply a), b) etc. – steve Jul 24 '19 at 22:18
  • 2
    @MontyHarder It depends. If there are a few hundred of these codes, each corresponding to a string, then another approach may be better. For the exact issue at hand, this is sufficient. – Kusalananda Jul 24 '19 at 22:18
  • 2
    @MontyHarder Sorry, I should have been clearer. By "code" I meant $CODE. I always call "exit status" exactly that, never just "code". If the script needs to use many hundreds of keys to refer to the strings, using a case statement becomes unwieldy. – Kusalananda Jul 24 '19 at 22:41
19

Assuming you are using bash release 4.0 or newer...

CODE=A

declare -A domain

domain=(
   [a]=com.tencent.ig
   [b]=com.vng.pubgmobile
   [c]=com.pubg.krmobile
   [d]=com.rekoo.pubgm
)

PN=${domain[${CODE,,}]:?ERROR! CODE KOSONG, MELAKUKAN EXIT OTOMATIS}

In the code, I define an associative array containing all the domain names, each associated with a single letter lower-case key.

The $PN variable is assigned the domain name corresponding to the lower-cased $CODE value (${CODE,,} returns the value of $CODE turned into lower case letters only) from this array, but if the $CODE does not correspond to a valid entry in the domain list, it exits the script with an error.

The ${variable:?error message} parameter substitution would expand to the value of $variable (the appropriate domain in the code) but would exit the script with the error message if the value is empty not available. You don't get exactly the same formatting of the error message as in your code, but it would essentially behave the same if $CODE is invalid:

$ bash script.sh
script.sh: line 12: domain[${CODE,,}]: ERROR! CODE KOSONG, MELAKUKAN EXIT OTOMATIS

If you care about character count, we can shorten this further:

CODE=A
declare -A domain=( [a]=tencent.ig [b]=vng.pubgmobile [c]=pubg.krmobile [d]=rekoo.pubgm )
PN=com.${domain[${CODE,,}]:?ERROR! CODE KOSONG, MELAKUKAN EXIT OTOMATIS}

Apart from deleting unnecessary newlines, I've also removed com. from each domain (this is instead added in the assignment to PN).

Note that all code above would work even for a multi-character value in $CODE (if lower-cased keys existed for these in the domain array).


If $CODE was a numerical (zero-based) index instead, this would simplify the code a bit:

CODE=0

domain=( com.tencent.ig com.vng.pubgmobile com.pubg.krmobile com.rekoo.pubgm )
PN=${domain[CODE]:?ERROR! CODE KOSONG, MELAKUKAN EXIT OTOMATIS}

This would additionally make it really easy to read the domain array from an auxiliary file containing one entry per line:

CODE=0

readarray -t domain <domains.txt
PN=${domain[CODE]:?ERROR! CODE KOSONG, MELAKUKAN EXIT OTOMATIS}
|improve this answer|||||
  • 1
    @IISomeOneII declare -A domain just says that domain should be an associative array ("hash") variable. – Kusalananda Jul 24 '19 at 7:14
  • 1
    @Isaac Now more distinct from yours. Thanks for the heads up. – Kusalananda Jul 24 '19 at 7:18
  • 1
    Would be better to use zsh or ksh93. For bash, you'd need a recent version and it would fail for empty values of $CODE. – Stéphane Chazelas Jul 24 '19 at 7:40
  • 1
    @StéphaneChazelas Yes, you would get one extra error message about a bad array subscript if $CODE was unset or empty, but it would still generate the correct custom error message after that. – Kusalananda Jul 24 '19 at 7:46
  • 1
    @Kusalananda A new (valid POSIX) script posted. Without the error checking is very short. – Isaac Jul 26 '19 at 23:00
11

If your shell allow arrays, the shortest answer should be like this example in bash:

declare -A site
site=( [a]=com.tencent.ig [b]=com.vng.pubgmobile [c]=com.pubg.krmobile [d]=com.rekoo.pubgm )

pn=${site[${code,}]}

That is assuming that $code could only be a, b, c or d.
If not, add a test like:

case ${site,} in
    a|b|c|d)        pn=${site[${code,}]};;
    *)              pn="default site"
                    printf '\a\t %s\n' 'ERROR!' 'CODE KOSONG' 'MELAKUKAN EXIT OTOMATIS'
                    exit 1
                    ;;
esac
|improve this answer|||||
  • If the input is A will it work on that script? Sorry my English bad – IISomeOneII Jul 24 '19 at 7:02
  • 2
    Yes, the expansion ${var,} converts to lowercase the first character of ${var}. @IISomeOneII – Isaac Jul 24 '19 at 7:07
  • 1
    ${var,} seems to be Bash-specific though. I think the associative array would work in ksh and zsh too – ilkkachu Jul 24 '19 at 7:08
  • @ilkkachu Yes, correct in both counts. – Isaac Jul 24 '19 at 7:12
  • Thx everyone, Lots of good people here 👍 – IISomeOneII Jul 24 '19 at 9:20
3

I'm going to take this answer a different direction. Rather than coding your data into the script, put that data into a separate data file, then use code to search the file:

$ cat names.cfg 
a com.tencent.ig
b com.vng.pubgmobile
c com.pubg.krmobile
d com.rekoo.pubgm

$ cat lookup.sh
PN=$(awk -v code="${1:-}" 'tolower($1) == tolower(code) { print $2; }' names.cfg)
if [ -z "${PN}" ]; then
  printf '\a\t%s\n' 'ERROR!' 'CODE KOSONG' 'MELAKUKAN EXIT OTOMATIS' >&2
  exit 1
fi
echo "${PN}"

$ bash lookup.sh A
com.tencent.ig
$ bash lookup.sh a
com.tencent.ig
$ bash lookup.sh x
    ERROR!
    CODE KOSONG
    MELAKUKAN EXIT OTOMATIS

Separating these concerns has a few benefits:

  • Add and remove data easily and simply, without having to work around code logic.
  • Other programs can reuse the data, like counting how many matches are in a particular sub domain.
  • If you have a huge list of data, you can sort it on disk and use look to efficiently binary search it (rather than line-by-line grep or awk)
|improve this answer|||||
  • 1
    If you go this way, you still need to arrange for PN to be set to the correct value. – ilkkachu Jul 26 '19 at 21:25
  • 1
    @ilkkachu Fair point. I missed that in the OP. Corrected. – bishop Jul 27 '19 at 0:03
  • 2
    +1 for separating data from code. – arp Jul 27 '19 at 0:48
1

You are using letters to index the values, if you were to use numbers, it becomes as simple as:

code=1
set -- com.tencent.ig com.vng.pubgmobile com.pubg.krmobile com.rekoo.pubgm

eval pn\=\${"$code"}

That's portable shell code, will work on most shells.
For bash you may use: pn=${!code}, or for bash/ksh/zsh use: pn=${@:code:1}.

letters

If you must user letters (from a to z, or A to Z) they must be converted to an index:

code=a                              # or A, B, C, ... etc.
set -- com.tencent.ig com.vng.pubgmobile com.pubg.krmobile com.rekoo.pubgm
eval pn\=\"\${$(( ($(printf '%d' "'$code")|32)-96  ))}\"

In a longer code to clarify the intent and meaning of each part:

code=A

set -- com.tencent.ig com.vng.pubgmobile com.pubg.krmobile com.rekoo.pubgm

asciival=$(( $(printf '%d' "'$code") ))      # byte value of the ASCII letter.
upperval=$(( asciival |  32 ))               # shift to uppercase.
indexval=$(( upperval -  96 ))               # convert to an index from a=1.
eval arg\=\"\$\{$indexval\}\"                # the argument at such index.

If you need to convert to lowercase values, use: $(( asciival & ~32 )) (make sure that bit 6 of the ascii value is unset).

error code

The output that your script prints on an error is quite long (and particular).
The most versatile way to deal with it is to define a function:

errorcode(){ exitcode=$1; shift; printf '\a\t %s\n' "$@"; exit "$exitcode"; }

And then call that function with the specific message(s) you need.

errorcode 27  "ERROR!" "CODE KOSONG" "MELAKUKAN EXIT OTOMATIS"

Note that the resulting exit value is given by exitcode (example here is 27).

A full script (with error checking) then becomes:

errorcode(){ exitcode=$1; shift; printf '\a\t %s\n' "$@"; exit "$exitcode"; }

code=${1:-A}

case "$code" in 
    [a-d]|[A-D]) : ;;
    *)           errorcode 27  "ERROR!" "CODE KOSONG" "MELAKUKAN EXIT OTOMATIS" ;;
esac

set -- com.tencent.ig com.vng.pubgmobile com.pubg.krmobile com.rekoo.pubgm
eval pn\=\"\${$(( ($(printf '%d' "'$code") & ~32) - 64  ))}\"

printf 'Code=%s Argument=%s\n' "$code" "$pn"
|improve this answer|||||

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.