我如何改进这个脚本？

Question

我正在学习shell脚本，我发现很难找到一个好的学习方法。我已经在下面创建了一个脚本，让用户通过选项搜索各种不同的互联网引擎。如果有人能看一看这篇文章，指出我做错了什么，如何改进等等，我将非常感激。

#!/bin/bash

## Get user search-engine option
while getopts aegwy: OPTIONS ; do
  case "$OPTIONS" in 
    a) ENGINE="http://www.amazon.com/s/ref=nb_sb_noss/?field-keywords";;
    e) ENGINE="http://www.ebay.com/sch/i.html?_nkw";;
    g) ENGINE="http://www.google.com/search?q";;
    w) ENGINE="http://en.wikipedia.org/wiki/?search";;
    y) ENGINE="http://www.youtube.com/results?search_query";;
    ?) ERRORS=true;;
  esac
done &>/dev/null

## Ensure correct command usage
[ $# -ne 2 ] || [ $ERRORS ] && printf "USAGE: $(basename $0) [-a Amazon] [-e eBay] [-g Google] [-w Wikipedia] [-y YouTube] \"search query\"\n" && exit 1

## Ensure user is connected to the Internet
ping -c 1 209.85.147.103 &>/dev/null ; [ $? -eq 2 ] && printf "You are not connected to the Internet!\n" && exit 1

## Reformat the search query
QUERY=`printf "$2" | sed 's/ /+/g'`

## Execute the search and exit program
which open &>/dev/null ; [ $? -eq 0 ] && open "$ENGINE"="$QUERY" &>/dev/null && exit 0 || xdg-open "$ENGINE"="$QUERY" &>/dev/null && exit 0 || printf "Command failed!\n" && exit 1

提前感谢大家，意义重大！

Answer 1

最好在codereviews中发布，如上所述，但这里有一些主要是风格上的评论。我应该强调的是，这个脚本非常好；这些只是一些很小的改进，我认为这些改进将有助于使代码更容易阅读/维护，在一些情况下更健壮，等等。

您不需要使用全大写的变量名，因为环境变量是全大写的；shell变量和环境变量不是一回事。

由于您的$OPTIONS变量一次只包含一个选项，因此最好使用单数名称(例如$option)。或者你可以使用$opt，这在这里有点传统。

getopts字符串中的: (aegwy:)表明-y选项需要一个参数，就像在-ysomething中一样，而不仅仅是-y本身。既然您没有对$OPTARG做任何事情，我猜这不是故意的。

正如其他人所说，if/then/elif/else可能比&&和||的链条更清晰。

测试[ $ERRORS ]有些不清楚，因为根据$ERRORS参数的内容，它可以表示许多不同的含义。一个更明确的指示，你只关心它是否被设置为[ -n "$ERRORS" ]。

像[ -ne ]和friends这样的比较大多是shell内置整数运算之前的遗留问题；更现代的习惯用法应该是(( $# != 2 ))。

您的用法消息暗示-a、-e、-g、-w和-y选项采用Amazon、eBay、Google等形式的参数。如果没有这些附加内容，命令的实际语法将更加清晰；您可以在帮助文本中包括一个额外的段落，列出每个选项的含义。

通常，错误消息应该发送到stderr，而不是stdout (>&2)。

为了保持输出的一致性，使用basename $0是很好的，但是不使用$0是有必要的，因为它将反映用户实际调用命令的方式。一些需要考虑的事情。

如果不使用格式字符串，则使用printf没有多大意义；只需使用echo，它会自动附加换行符。用法消息传统上也不包括引号；引用arg与否取决于用户是否需要它。

检查命令是否成功正是if的工作方式，因此没有必要对$?进行显式检查，除非您真正关心确切的退出值。在连接ping的情况下，您可能不关心它失败的原因，只关心它失败了：

  if ! ping -c 1 209.85.147.103 >/dev/null; then 
     echo >&2 "$0: You are not connected to the Internet!"
     exit 1
  fi

您的搜索查询重新格式化可能需要做的不仅仅是将空格转换为加号；如果它有一个与号怎么办？但是如果你只是在做空格到加号的事情，你可以使用bash参数扩展，不需要sed：QUERY="${QUERY// /+}"

如果您的程序依赖于open/xdg-open等，那么您可能应该在顶部检查它的可用性；如果您知道无论如何都不可能执行请求的操作，那么做其他任何事情都没有意义。你可以使用一个变量，这样你就不会在多个子句中重复：

open=
for cmd in open xdg-open; do
   if type -p "$cmd" >/dev/null; then
     open="$cmd"
     break
   fi
done
if [ -z "$open" ]; then
   echo >&2 "$0: open command not found."
   exit 1
fi

然后，您可以只使用这一行来结束：

"$open“"$ENGINE=$QUERY”&>/dev/null

Answer 2

http://linuxcommand.org/是一个极好的资源，可以提高您的bash脚本编写技能。

http://tldp.org/LDP/abs/html/是另一个很棒的文档。

希望这能有所帮助。