# Bad performance bash, changing display brightness

I have a script that changes the display brightness using xrandr. It is my first bash script. I mainly had a lot of trouble with dealing with the floating point values. I think that is one of the main reasons the performance is bad.

  1 #!/bin/bash
2 # increase/decrease display brightness of xrandr of both displays
3 # use:
4 # $./change_brightness [up/down (0/1)] 5 6 export current_brightness=$(xrandr --verbose | awk '/Brightness/ { print $2; exit }') 7 8 export increase=$1
9 export new_brightness=$current_brightness 10 11 if [ "$increase" = "1" ]; then
12     if [ $current_brightness != "1.0" ]; then 13 new_brightness="$current_brightness + 0.2"
14     fi
15 elif [ "$increase" = "0" ]; then 16 if [$current_brightness != "0.20" ]; then
17         new_brightness="$current_brightness - 0.2" 18 fi 19 fi 20 21 new_brightness=$(bc <<< "$new_brightness") 22 23 xrandr --output eDP-1-1 --brightness$new_brightness
24 xrandr --output DP-0 --brightness $new_brightness  The performance isn't terrible, but any improvements would be highly appreciated. Also the improvements that have nothing to do with performance. • Please don't put line numbers in code snippets. That makes it much harder for people like me to cut and paste your code – glenn jackman Nov 25 '18 at 14:50 ## 2 Answers I'm confused about your goal for this question: 1. performance is bad 2. performance isn't terrible 3. improvements would be appreciated 4. improvements have nothing to do with performance. Nevertheless, code review notes: • you don't have to export every variable, only the ones that are needed by processes you spawn from this script. • validate inputs: why do anything if the input is neither "0" nor "1"? Here's my take: #!/bin/bash # increase/decrease display brightness of xrandr of both displays # use: #$ ./change_brightness [up/down (0/1)]

current_brightness=$(xrandr --verbose | awk '/Brightness/ { print$2; exit }')
case $1 in 0) direction=-1;; 1) direction=1;; *) echo "some error message"; exit 1;; esac export current_brightness direction new_brightness=$( perl -E '
use List::Util qw( min max );
say max(0, min($ENV{current_brightness} +$ENV{direction} * 0.2, 1.0));
')

xrandr --output eDP-1-1 --brightness $new_brightness xrandr --output DP-0 --brightness$new_brightness


As @glenn-jackman pointed out, validate your inputs. If the script argument should be either 0 or 1, then enforce that rule, by checking the value, and exit with an error if it's some other value. It's always good to fail early, for example to avoid confusing crashes later due to an unexpected value, or to avoid doing unnecessary work.

### Look for corner cases

The script increases or decreases brightness in increments of 0.2, unless the value is 1.0 or 0.2. This is fragile. What if the current value is 0.9? The script will not prevent going to 1.1, 1.3, ..., or 0.1, -0.1, ..., and so on. This is a bug waiting to happen.

To correctly enforce upper and lower limits, you need an appropriate technique. As you discovered, Bash cannot do floating point math. I would recommend Awk instead. It's lightweight and universally available.

### Always double-quote variables in program arguments

As a good rule of thumb, always double-quote variables used in program arguments, for example here:

if [ $current_brightness != "1.0" ]; then ... xrandr --output eDP-1-1 --brightness$new_brightness


if [ "$current_brightness" != "1.0" ]; then ... xrandr --output eDP-1-1 --brightness "$new_brightness"