2

私はシェル スクリプトを学んでいますが、良い学習方法を見つけるのが難しいと感じています。ユーザーがさまざまなインターネット エンジンをオプションで検索できるようにするスクリプトを以下に作成しました。誰かがこれを見て、私が間違っていること、それを改善する方法などを指摘できれば、本当に感謝しています.

#!/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

よろしくお願いします。

4

2 に答える 2

2

上記のように、コードレビューに投稿するのが最適ですが、主に文体に関するコメントがいくつかあります。スクリプトはそのままで問題ないことを強調しておきます。これらは、コードの読み取り/保守を容易にし、いくつかのケースでより堅牢にするのに役立つと思われる小さな改善です。

環境変数がすべて大文字だからといって、変数名にすべて大文字を使用する必要はありません。シェル変数と環境変数は同じものではありません。

$OPTIONS変数は一度に 1 つのオプションしか保持しないため、単数形の名前の方が適しています (例: ) $option。$optまたは、ここではやや伝統的な を使用することもできます。

:getopts 文字列 ( aegwy:) の は、オプション-yが引数を期待し-yていることを示しています-y。で何もしていないので$OPTARG、意図的ではないと思います。

他の人が言ったように、// /ifはthenおそらくandのチェーンよりも明確です。elifelse&&||

パラメータ[ $ERRORS ]の内容に応じてさまざまなことを意味する可能性があるため、テストはやや不明確です。$ERRORS設定されているかどうかだけを気にすることをより明確に示すと、 になります[ -n "$ERRORS" ]。

[ -ne ]likeと friends の比較は、ほとんどがシェルに整数演算が組み込まれる前からの名残りです。より現代的なイディオムは(( $# != 2 )).

使用法メッセージは、-a、-e、-g、-w、および -y オプションが Amazon、eBay、Google などの形式の引数を取ることを意味します。これらの追加がなければ、コマンドの実際の構文がより明確になります。 ; 各オプションの意味をリストしたヘルプ テキストに追加の段落を含めることができます。

原則として、エラー メッセージは stdout ではなく stderr に送信する必要があります ( >&2)。

basename $0出力の一貫性のために使用しても問題あり$0ませんが、ユーザーが実際にコマンドを呼び出したことが反映されるため、放っておくと言いたいことがあります。考慮すべきこと。

printfフォーマット文字列を使用していない場合は、あまり意味がありません。を使用するだけechoで、自動的に改行が追加されます。使用法メッセージには、伝統的に引用符も含まれていません。引数が必要かどうかに応じて、引数を引用するかどうかはユーザー次第です。

コマンドの成功をチェックすることはまさにその仕組みなので、正確な終了値を本当に気にしない限り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

検索クエリの再フォーマットでは、スペースをプラス記号に変換するだけでは不十分な場合があります。アンパサンドがある場合はどうなりますか？ただし、スペースをプラスに変換するだけの場合は、sed を使用せずに bash パラメーター展開を使用できます。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

その後、次の 1 行だけで仕上げることができます。

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

于 2012-04-13T20:43:12.350 に答える
0

http://linuxcommand.org/は、bash スクリプトのスキルを向上させるための優れたリソースです。

http://tldp.org/LDP/abs/html/も素晴らしいドキュメントです。

お役に立てれば。

于 2012-04-12T07:06:51.837 に答える