Pattern: Unnecessary echo
Issue: -
You appear to be using echo to write a value to stdout, and then using $(..) or `..` to capture the value again. Consider replacing $(echo somevalue) with somevalue.
Example of incorrect code:
greeting=$(echo "Hello, $name")
# or
tar czf "$(echo "$(date +%F).tar.gz")" *Example of correct code:
greeting="Hello, $name"
# or
tar czf "$(date +%F).tar.gz" *Sometimes this pattern is used because of side effect of echo or expansions. For example, here $(echo ..) is used to expand a glob.
glob="*.png"
files="$(echo $var)"
The echo is not useless, but this code is problematic because it concatenates filenames by spaces. This will break filenames containing spaces and other characters later when the list is split again. Better options are:
- Arrays, if supported by the shell:
files=( $glob ); echo "The first file is ${files[0]}" - Positional parameters when possible:
set -- $glob; echo "The first file is $1" - Delaying expansion until it's needed:
for file in $glob; do ...
All three methods will let you avoid issues with special characters in filenames.
As another example, here $(echo ..) is used to expand escape sequences:
unexpanded='var\tvalue'
expanded="$(echo "$var")"
In this case, use printf instead. It's well defined with regard to escape sequences.
Finally, if you really do want to concatenate a series of elements by a character like space, consider doing it explicitly with for or printf (e.g. printf '%s\n' $glob).