이 스크립트를 어떻게 향상시킬 수 있습니까?

Question

쉘 스크립팅을 배우고 있으며 배우기 좋은 방법을 찾는 것이 어렵다는 것을 알고 있습니다. 아래 옵션을 통해 사용자가 다양한 인터넷 엔진을 검색 할 수있게 해주는 스크립트를 만들었습니다. 어떤 사람은이 문제를보고 내가 잘못 것을 지적 할 수 있다면 내가 어떻게

#!/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에 게시되었지만 대부분 여기에 약간의 문체가 있습니다. 나는 그 대본이있는 그대로 잘 유지된다는 것을 강조해야한다. 이것들은 단지 코드를 읽기/유지하기가 더 쉽고, 몇 가지 경우에 더 견고해질 수 있도록 도와주는 작은 개선 사항입니다.

환경 변수가 다음과 같기 때문에 변수 이름에 모두 대문자를 사용할 필요는 없습니다. 모든 대문자; 쉘 변수와 환경 변수가 같은 것은 아닙니다.

변수 $OPTIONS 변수는 한 번에 하나의 옵션 만 보유하므로 단 하나의 이름 (예 : $option)이 좋습니다. 또는 $opt으로 갈 수도 있습니다. 다소 전통적입니다. -y 옵션은 인수를 기대하는 당신의 getopts가 문자열 (aegwy:)에서

:은 그 자체로 대신 -y의 -y뭔가 같이 나타냅니다. $OPTARG으로 아무 것도하지 않으므로 의도적이지 않습니다. 다른 사람이 말했듯이

, if/then/elif/else 아마 && 및 ||의 사슬보다 더 명확하게 될 것이다.

테스트 [ $ERRORS ]은 $ERRORS 매개 변수의 내용에 따라 많은 다른 것들을 의미 할 수 있기 때문에 다소 명확하지 않습니다. 설정 여부 만 신경 쓰면 더 명확한 표시가 [ -n "$ERRORS" ]이됩니다.

[ -ne ]과 같은 비교는 쉘이 내장 정수 연산을하기 전에 대부분 홀수가됩니다. 보다 현대적인 숙어는 (($# != 2))이 될 것입니다.

사용 메시지는 -a, -e, -g, -w 및 -y 옵션이 Amazon, eBay, Google 등의 형식의 인수를 사용한다는 것을 의미합니다. 명령의 실제 구문 추가하지 않아도됩니다. 도움말 텍스트에 각 옵션의 의미를 나타내는 추가 단락을 추가 할 수 있습니다.

일반적으로 오류 메시지는 stdout (>&2) 대신 stderr로 이동해야합니다.

출력의 일관성을 위해 basename $0을 사용하는 것이 좋지만, 사용자가 실제로 명령을 실행했음을 반영하기 때문에 $0을 그대로 두는 것이 좋습니다. 고려해야 할 것.

형식 문자열을 사용하지 않는 경우 printf을 사용하면 별다른 의미가 없습니다. echo을 사용하면 자동으로 줄 바꿈이 추가됩니다. 사용 메시지에는 전통적으로 따옴표가 포함되지 않습니다. arg의 인용 여부는 사용자가 결정합니다.

성공을위한 명령을 확인하는 것은 정확히 if의 작동 방식이므로 정확하게 종료 값을 신경 쓰지 않는 한 $?의 명시 적 검사를 수행 할 필요가 없습니다. 당신의 연결 핑의 경우, 당신은 아마 한 것만, 실패한 이유에 대해 걱정하지 않는다 :

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

검색 쿼리를 다시 포맷 그냥 더하기 기호로 공간을 설정보다 더 많은 일을해야 할 수도 있습니다; 앰퍼샌드가 있다면 어떨까요? 그러나 만약 당신이 space-to-pluses 일을한다면 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 

그리고 당신은 단지이 하나 개의 라인으로 마무리 할 수 ​​있습니다 : 여러 조항에서 자신을 반복 바람을하지 않아도 그리고 당신은 변수를 사용할 수 있습니다 = $ QUERY "&>/dev/null

Answer 2

http://linuxcommand.org/은 bash 스크립팅 기술을 향상시키는 데 유용한 자료입니다.

http://tldp.org/LDP/abs/html/은 훌륭한 문서입니다.

희망이 도움이됩니다.