LINUX.ORG.RU

Что можно улучшить в данном скрипте?

 , devel, ,


0

1

Шалом.

Сабж. Может что-то можно оптимизировать (для скорости работы, удобства чтения и ПРАВИЛЬНОСТИ)?

#!/bin/sh

# CPUs
getstat_core_temp_separator="/"
getstat_core_temp_suffix="°C"

# Battery
getstat_battery_state_charging="заряжается"
getstat_battery_state_discharging="разряжается"
getstat_battery_state_charged="заряжена"
getstat_battery_state_missing="отсутсвует"
getstat_battery_state_unknown="ошибка"


getstat_sysctl="sysctl -n"
getstat_arg_n="$#"

getstat_core_temperature() {
    getstat_core_n="`${getstat_sysctl} hw.ncpu`"
    while [ ! "$getstat_core_n" -eq "0" ] ; do
	getstat_core_n="$(( ${getstat_core_n} - 1 ))"
	getstat_core_temp="`${getstat_sysctl} dev.cpu.${getstat_core_n}.temperature`"
	getstat_core_temp="`printf "${getstat_core_temp}" | cut -d, -f1`"
	printf "${getstat_core_temp}${getstat_core_temp_suffix}"
	[ "$getstat_core_n" -ge "1" ] && printf "${getstat_core_temp_separator}"
    done
}

getstat_memory_total() {
    getstat_memory_page_size="`${getstat_sysctl} vm.stats.vm.v_page_size`"
    getstat_memory_page_count="`${getstat_sysctl} vm.stats.vm.v_page_count`"
    getstat_memory_total="$(( ${getstat_memory_page_count} * ${getstat_memory_page_size} / 1048576 ))"
    printf "${getstat_memory_total}"
}

getstat_memory_used() {
    getstat_memory_page_size="`${getstat_sysctl} vm.stats.vm.v_page_size`"
    getstat_memory_page_wire="`${getstat_sysctl} vm.stats.vm.v_wire_count`"
    getstat_memory_page_active="`${getstat_sysctl} vm.stats.vm.v_active_count`"
    getstat_memory_used="$(( (${getstat_memory_page_wire} + ${getstat_memory_page_active}) * \
${getstat_memory_page_size} / 1048576 ))"
    printf "${getstat_memory_used}"
}

getstat_memory_free() {
    getstat_memory_free="$(( `getstat_memory_total` - `getstat_memory_used` ))"
    printf "${getstat_memory_free}"
}

getstat_battery_state() {
    getstat_battery_state="`${getstat_sysctl} hw.acpi.battery.state`"
    if [ "$getstat_battery_state" -eq "2" ] ; then
	getstat_battery_state="${getstat_battery_state_charging:=charging}"
    elif [ "$getstat_battery_state" -eq "1" ] ; then
	getstat_battery_state="${getstat_battery_state_discharging:=discharging}"
    elif [ "$getstat_battery_state" -eq "0" ] ; then
	getstat_battery_state="${getstat_battery_state_charged:=charged}"
    elif [ "$getstat_battery_state" -eq "-1" ] ; then
	getstat_battery_state="${getstat_battery_state_missing:=missing}"
    else
	getstat_battery_state="${getstat_battery_state_unknown:=unknown}"
    fi
    printf "${getstat_battery_state}"
}

getstat_battery_life() {
    getstat_battery_life="`${getstat_sysctl} hw.acpi.battery.life`"
    printf "${getstat_battery_life}"
}

while [ "$getstat_arg_n" -ne "0" ] ; do
    getstat_arg_n="$(( ${getstat_arg_n} - 1 ))"
    case ${1} in
	-ct)	getstat_out="${getstat_out} `getstat_core_temperature`" ;;
	-mt)	getstat_out="${getstat_out} `getstat_memory_total`" ;;
	-mu)	getstat_out="${getstat_out} `getstat_memory_used`" ;;
	-mf)	getstat_out="${getstat_out} `getstat_memory_free`" ;;
	-bs)	getstat_out="${getstat_out} `getstat_battery_state`" ;;
	-bl)	getstat_out="${getstat_out} `getstat_battery_life`" ;;
	*)	getstat_out="${getstat_out} `printf "${1}"`" ;;
    esac
    shift
done

echo ${getstat_out}


// Это не конец, будет больше отдавать.

Интересует улучшение конструкции обработки параметров в первую очередь.

Заранее благодарю.

★★★★★

Последнее исправление: IPR (всего исправлений: 1)

Это тот случай, когда лучше это переписать на какой-нибудь питон или перл. Да и вообще, юзкейс скрипта какой? Заменить уже имеющиеся утилиты для отображения этой информации? Или там, куда ты собираешь его впихать, их нет?

anonymous
()

Вроде только мелочи вроде

printf «${getstat_battery_state}»

case ${1} in #quotes?

Ну и #!/bin/sh не нужен, достаточно chmod +x - shell это интерпретатор по умолчанию(хотя трудно представляю себе систему, на которой бинарь shell был бы не /bin/sh)

улучшение конструкции обработки параметров

Параметров скрипта имелось ввиду? Там всё норм

Deleted
()
Последнее исправление: dllmain (всего исправлений: 1)
Ответ на: комментарий от anonymous

Это тот случай, когда лучше это переписать на какой-нибудь питон или перл.

Зачем? Чтобы потом страдать, когда сломают синтаксис (как в Python 2/3)?

Да и вообще, юзкейс скрипта какой? Заменить уже имеющиеся утилиты для отображения этой информации? Или там, куда ты собираешь его впихать, их нет?

Polybar не умеет.

IPR ★★★★★
() автор топика
Ответ на: комментарий от Deleted

Вроде только мелочи вроде printf «${getstat_battery_state}» case ${1} in #quotes?

Что с ними? Объясни, плиз.

IPR ★★★★★
() автор топика
Ответ на: комментарий от IPR

printf format [argument...]

Попадётся метасимвол(%) в первом аргументе - будет ошибка на выходе. Юзай ptintf '%s' "${arg}". А в case кавычки забыл, наверное? В остальных местах проставлены вроде

Deleted
()
Ответ на: комментарий от IPR

Чтобы потом страдать, когда сломают синтаксис (как в Python 2/3)?

Я, как человек, писавший для обоих версий и до сих пор поддерживающий большой набор скриптов, которые работают в обоих версиях, всегда недоумеваю подобным комментам.

Если у тебя такие проблемы с чтением документации и написанием переносимого кода, что это доставляет тебе моральные и физические страдания, почему ты вообще паришься о чистоте (!) bash (!) кода?

E ★★★
()

elif'ы эти можно на case переписать, это будет логичнее\негляднее (а вообще в нормальном ЯП я бы это сделал на уровне ключей массива, не знаю можно ли так в bash, ну да ладно).

Вместо того чтоб стопицот раз дергать sysctl можно сделать это один раз - можно указать несколько нужных имен и маппнуть потом на список имен, получив массив.

Вызовы внешних команд я бы не через тильду делал, а в $(), тем более что для математики у вас уже это используется.

Аргументы с одним дефисом общепринято состоят из одной буквыб не вижу причин тут так не сделать.

Раз уж sysctl вызываете через переменную - стоит прописать полный путь, желательно полученный через whereis или аналогичным способом.

Так как скрипт будет работать только от рута (точнее вызов sysctl должен быть от рута) не плохо бы вкорячить проверку на это.

Вместо принта ключа, для которого нет действия, нужно показывать какой-нибудь вариант help'а или хоть тупо сообщение что такого ключа нет.

Постоянные getstat из имен можно выкинуть - других действий у вас нет, не за чем захламлять скрипт.

micronekodesu ★★★
()
Ответ на: комментарий от E

паришься о чистоте (!) bash (!) кода?

Не bash, а shell. У меня bash'а даже в системе нет.

И да, на sh код пишется и о нём забывается, он просто работает. Не нужно потом лезть и исправлять print на print() и т.д. и т.п.

IPR ★★★★★
() автор топика
Ответ на: комментарий от micronekodesu

elif'ы эти можно на case переписать, это будет логичнее\негляднее (а вообще в нормальном ЯП я бы это сделал на уровне ключей массива, не знаю можно ли так в bash, ну да ладно). Вместо того чтоб стопицот раз дергать sysctl можно сделать это один раз - можно указать несколько нужных имен и маппнуть потом на список имен, получив массив.

Это не bash, тут массивов нет.

Вызовы внешних команд я бы не через тильду делал, а в $(), тем более что для математики у вас уже это используется.

А какая разница? Я серьёзно не знаю.

Аргументы с одним дефисом общепринято состоят из одной буквыб не вижу причин тут так не сделать.

Не всегда, в иксах, например, это не так. И да, в одну букву в будущем не уложусь, увы.

Раз уж sysctl вызываете через переменную - стоит прописать полный путь, желательно полученный через whereis или аналогичным способом.

Это верно, учту. Я обычно which cmdname делаю.

Так как скрипт будет работать только от рута (точнее вызов sysctl должен быть от рута) не плохо бы вкорячить проверку на это.

А это точно? Я просто от юзера вызываю, но у меня юзер в operator и wheel состоит. Надо проверить от nobody.

Вместо принта ключа, для которого нет действия, нужно показывать какой-нибудь вариант help'а или хоть тупо сообщение что такого ключа нет.

Это нужно для вывода текста в некоторых барах. Например getstat Свободно памяти: -mf Имхо, так удобнее. При запуске без аргументов будет вываливаться хелп.

Постоянные getstat из имен можно выкинуть - других действий у вас нет, не за чем захламлять скрипт.

Привычка, не хочу пересечения с какими-нибудь внешними переменными (вдруг кто-то кроме меня будет использовать?)

IPR ★★★★★
() автор топика
Ответ на: комментарий от Deleted

Это не bash, это posix shell код. Я, по крайней мере, не увидел расширений

Всё верно.

IPR ★★★★★
() автор топика

Удивительно, но это тот самый момент, когда улучшать можно каждую строку.

В кавычках при присваивании надо брать только спец-символы. Ну никак / или $# их не требует.

То же касается условий [ $var -eq ... ] — если у вас в $var не число, то скрипт всё равно будет работать чёрте как с ошибками и никакого смысла там кавычек нет. Совсем смешно уже ваши "0".

Если хотите скорости, то заведите глобальные переменные как результат функций, а не делайте `функция` с printf.

vodz ★★★★★
()
Ответ на: комментарий от vodz

В кавычках при присваивании надо брать только спец-символы. Ну никак / или $# их не требует.

То же касается условий [ $var -eq ... ] — если у вас в $var не число, то скрипт всё равно будет работать чёрте как с ошибками и никакого смысла там кавычек нет. Совсем смешно уже ваши «0».

Учту. Хотя я это знаю, но вот такая привычка выработалась. Всё, блджад, в кавычки засовываю.

IPR ★★★★★
() автор топика
Ответ на: комментарий от IPR

но вот такая привычка выработалась. Всё, блджад, в кавычки засовываю.

Правильная привычка. В данном случае действительно большого смысла нет. А вот если бы стояло не [ $var -eq 0 ], а [ $var = 0 ], то смысл был бы. А т. к. кавычки по-любому не мешают, то зачем вырабатывать новые дурные привычки?

aureliano15 ★★
()
Ответ на: комментарий от aureliano15

а [ $var = 0 ], то смысл был бы. А т. к. кавычки по-любому не мешают, то зачем вырабатывать новые дурные привычки?

Это уже не bash, ибо в bash правильно [[ $var = 0 ]].

А т. к. кавычки по-любому не мешают, то зачем вырабатывать новые дурные привычки?

Что за глупость? Привычки должны быть правильными, если вы пишите смешное «0», то это дурная привычка — бессмысленное, мусорное, без обдумывания написание.

vodz ★★★★★
()
Ответ на: комментарий от vodz

а [ $var = 0 ]

Это уже не bash, ибо в bash правильно [[ $var = 0 ]].

В bash правильно и так, и так, но тс пишет не на bash, а [[ есть только в bash.

А т. к. кавычки по-любому не мешают, то зачем вырабатывать новые дурные привычки?

Что за глупость? Привычки должны быть правильными, если вы пишите смешное «0», то это дурная привычка — бессмысленное, мусорное, без обдумывания написание.

Очень часто рутинные вещи делаются на автомате, не задумываясь. Поэтому, имхо, лучше писать более безопасно, особенно если это практически ничего не стоит. И особенно при написании скриптов, где большинство ошибок появляются во время выполнения, и компилятор не указывает, что де в этой строке вы допустили такую-то ошибку, а вместо этого оболочка пишет об отсутствии операнда, который, вроде бы, на месте. Просто экономия времени и нервов.

aureliano15 ★★
()
Ответ на: комментарий от Deleted

Поэтому ОП его и не использует, да? Тупак на самом деле, башизмы быстрее и лучше работают. Да и удобно на самом деле.

anonymous
()
Ответ на: комментарий от anonymous

башизмы быстрее и лучше работают.

Сам по себе bash — не самая лёгкая оболочка. Поэтому если скрипт загружается по расписанию каждые несколько секунд, например, то по возможности лучше использовать более лёгкую оболочку.

aureliano15 ★★
()
Ответ на: комментарий от anonymous

Мне неизвестны причины ТС-а. По поводу более быстрой и лучшей работы - не встречал кейсов окромя регулярок в комплекте и массивов.

Deleted
()

Переписать с шелла на какой-нибудь язык программирования

annulen ★★★★★
()

Не надо использовать обратные апострофы.

Не надо писать подобные вещи на шелле.

Deleted
()
Ответ на: комментарий от aureliano15

В bash правильно и так, и так, но тс пишет не на bash, а [[ есть только в bash.

Нет. Правильно либо так [ «$var» = 0 ] либо так [[ $var = 0 ]]. В тегах стоит bash. [[ — придумано не в bash, это не чистый башизм.

Просто экономия времени и нервов.

Ваша проблема даже не в том, что вы тут сделали могобуквенное оправдание своей лени вдумываться в написуемый код. Кавычки указывают на особенность применённого алгоритма: тут могут быть, но не нельзя парсить спец-символы. Когда читаешь хороший код такие комментарии в коде не требуются. А вот для вашей дурацкой привычки каждый раз ищешь, где тут может быть пустая строка или маска и что в конечном итоге получится.

vodz ★★★★★
()
Ответ на: комментарий от Deleted

[[ ]] и $'\0' забыл, и остальные встроенные фичи — вызовы софта по-умолчанию медленней и хуже, особенно в цикле.

anonymous
()
Ответ на: комментарий от Deleted

Точно. Но всё-таки bash - это не про скорость, как ни крути

Ну дык, какая скорость, если авторы как тут юзают cut для получения первого поля. Ладно хоть не awk...

vodz ★★★★★
()
Ответ на: комментарий от vodz

Нет. Правильно либо так [ «$var» = 0 ]

Так я об этом же и написал, что если б стояло =, а не -eq, то без кавычек — неправильно.

Кавычки указывают на особенность применённого алгоритма: тут могут быть, но не нельзя парсить спец-символы. Когда читаешь хороший код такие комментарии в коде не требуются. А вот для вашей дурацкой привычки каждый раз ищешь, где тут может быть пустая строка или маска

С этим соглашусь.

aureliano15 ★★
()
Ответ на: комментарий от Deleted

Я только так бы заменить смог: «${VAR%%$delim*}». А какие ещё варианты есть?

IFS=$delim read f1 other. Или по-вашему надо в ситаксисе для этого делать большую избыточность? Ну вот вам cut/awk/sed :) Самый смак в том, что cut не умеет два пробела в качестве delim, потому некоторые скрипты могут ломаться на ровном месте, стоит лишь внешней программе нарисовать дополнительные пробелы для красоты.

vodz ★★★★★
()
Ответ на: комментарий от Deleted

Он хорош, но перегружен фичами и медленен. Но как же он удобен, можно кратко записать много интересного (он больше про интерактивную сессию конечно).

А без баша по-моему даже 2 числа нормально не сложить (хотя всё равно bc приходится почему-то наворачивать).

anonymous
()
Ответ на: комментарий от anonymous

А без баша по-моему даже 2 числа нормально не сложить

Вот это $((expression)) теперь в стандарте(с 2013, наверное). Для несложных случаев сойдёт

Deleted
()
Ответ на: комментарий от Deleted

Вот это $((expression)) теперь в стандарте(с 2013, наверное). Для несложных случаев сойдёт

У меня на эту тему есть смешная история. В busybox-ash мы сделали парсер арифметических выражений (моё там присваивания типа = += ++, всякие ?:, запятые и прочие **), понимающий в стиле bash-а перменные без $ и рекурсивно добираясь до числа. Я его предложил оригинальному автору. Ответ его меня искренне поразил: он хорош для busybox, так как маленький и bash-совместимый, но в (d)ash я его вставлять не буду. Я долго думал, и пока сколняюсь к мнению, что просто код сильно брайнфаковский получился и он его не понял.

vodz ★★★★★
()
Ответ на: комментарий от anonymous

Скорее всего, ради posix-compliant впилил

Deleted
()
Ответ на: комментарий от Deleted

Сейчас и в ash(судя по манам), и в dash оно таки есть

Да, давно, но вот 5.9 не понимает ++, нет, оно ругается на a++, а на ++a оно не ругается, но просто рассматривает как два подряд унарных минуса, то ест даже не то что нереализованная фича, а просто ошибка.

vodz ★★★★★
()
Ответ на: комментарий от anonymous

Перл я ещё могу понять, но питон для таких вещей не годится абсолютно. Пайпы и прочее там используются через жопу, на баше это проще и лаконичней. Это как на задачу написать один единственный SQL-запрос предлагать орм.

WitcherGeralt ★★
()
Ответ на: комментарий от IPR

Привычка, не хочу пересечения с какими-нибудь внешними переменными (вдруг кто-то кроме меня будет использовать?)

local

anonymous
()

Во-первых перый аргумент printf - это ФОРМАТ, а не данные. Данные (особенно приходящие извне) идут ПОСЛЕ. Это вопрос безопасности.

Переменная getstat_arg_n не нужна вообще.

Вот такие вещи можно писать короче:

getstat_core_temp="`${getstat_sysctl} dev.cpu.${getstat_core_n}.temperature`"
getstat_core_temp="`printf "${getstat_core_temp}" | cut -d, -f1`"
printf "${getstat_core_temp}${getstat_core_temp_suffix}"

например, так:

printf "%s%s" \
    "`${getstat_sysctl} dev.cpu.${getstat_core_n}.temperature | cut -d, -f1`" \
    "${getstat_core_temp_suffix}"

Некоторые промежуточные переменные нужны, но их мало, остальные просто мусор.

Ибо эти твои перекладывания из переменной в переменную не добавляют коду понятности, но делают его длиннее.

В функции getstat_battery_state вместо уродливой цепочки if можно использовать ассоциативный массив

declare -A getstat_battery_states
getstat_battery_states=(
    [-1]="missing"
    [0]="charged"
    [1]="discharging"
    [2]="charging"
)
echo -n "${getstat_battery_states[`${getstat_sysctl} hw.acpi.battery.state`]:-unknown}"
legolegs ★★★★★
()
Последнее исправление: legolegs (всего исправлений: 2)
Ответ на: комментарий от RazrFalcon

Ровно те же что и у забивания гвоздей молотком вместо плоскогубцев.

WitcherGeralt ★★
()
Ответ на: комментарий от WitcherGeralt

нормально всё в питоне с пайпами.

на задачу написать один единственный SQL-запрос предлагать орм

только написать вместо одной строки придётся 5, а sqlalchemy например можно и без orm юзать со всеми приятными фичами типа трансляции, верификации, и эмуляции нормальных типов (некоторые движки лишены нормальных типов) — что же теперь, отказываться?

тут стоило бы подумать, а нужны ли вообще базы данных тут, и следом а не превратится ли этот 1 sql запрос в 100 уже завтра.

anonymous
()
Ответ на: комментарий от RazrFalcon

А есть причины использовать bash?

Откуда вы, блджад, лезете?
1. Это не bash
2. POSIX Shell есть везде
3. Будет работать всегда

IPR ★★★★★
() автор топика
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.