HiveBrain v1.2.0
Get Started
← Back to all entries
patternbashMinor

Optimising script that does simple renaming of files

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scriptsimpleoptimisingfilesthatrenamingdoes

Problem

I have recently written some code in Bash where I'm starting to program some things and I think I'm overcomplicating the script I wrote.

I'd like to make it simpler so that in future I would avoid these mistakes.

I've posted this on Stack Overflow and one user did reply that I can "simplify" multi-echoing:


just use 1 echo command, open with a double quote on the first line and close the quote after the last one

Here's the code:

```
#!/bin/bash

# Look for path when the skript is
SCRIPT_PATH=$(readlink -f "$0")

SEARCH=$(dirname "$SCRIPT_PATH")"/"
REPLACE="../../"

echo
echo \ \ START - Replacing Absolute path to Relative
echo \ \ =================================================================
echo \ \ Info: Automatic replacement of the \>absolute\relative\> $SEARCH"
echo \ \ "Replacing by >> $REPLACE"
echo
echo \ \ "Found in files:"
echo \ \ =================
echo

# grep - filter files where inside is scripts path name and write them down
grep -rl "${SEARCH}" --include \*.pc ./

# replace them with sed ../../
grep -rl "${SEARCH}" --include \*.pc ./ | xargs sed -i "s#${SEARCH}#${REPLACE}#g"

echo
echo \ \ END - Replacing finished...
echo \ \ =================================================================
echo \ \ =================================================================
echo
echo
echo \ \ START - Renaming Folders by Y-Coordinate of Impactor
echo \ \ =================================================================
echo \ \ Info: Script will look into RESULTS Folder, than find directories
echo \ \ then find files in these directories and from
echo \ \ file named POSITIONING extcarc Y-Coordinate of the Impactor
echo \ \ This value is then used to rename by the: MODEL_Y\$Y-Coord
echo \ \ =================================================================
echo

# $SEARCH=path where the script is + /RESULTS/ where the results are
adresar=$SEARCH"RESULTS/"

directories=$(find $adresar*/ -type d)
echo
echo "Folders in dir: $adresar:"
echo

for d in $

Solution

Simplifying the echo statements

Your question on SO suggests that you're particularly interested in simplifying all those echo statements.
A feature called here-documents can help you, for example:

cat absoluterelative>  $SEARCH
  Replacing by  >>  $REPLACE

  Found in files:
  =================
EOF


Notice that this way you can also omit many quotes and escapes.

Speaking of quoting, sometimes you quote too much.
For example, instead of this:

SEARCH=$(dirname "$SCRIPT_PATH")"/"


You could write:

SEARCH=$(dirname "$SCRIPT_PATH")/


Danger! Watch out for falling rocks!

Storing the output of find and then iterating over it is error prone.
I mean this kind of code:

directories=$(find $adresar*/ -type d)

for d in $directories


The problem with this is that it won't work with subdirectories that have a space in their name. This is because in the for loop, the shell splits the value of $directories by any whitespace character (space, tab, newline), not only newline, as in the output of find.

For example if you have a subdirectory named "a b", find will output it on its own line,
but when you iterate over $directories, the shell will split the line of "a b" in the middle.

This splitting is defined by the variable IFS,
you can read about it in man bash.

One way to make your commands safe is to set IFS to newline, like this:

oldIFS="$IFS"
IFS=\n'

directories=$(find $adresar*/ -type d)

for d in $directories; do
    # do work ...
done

IFS="$oldIFS"

Code Snippets

cat <<EOF
  START - Replacing Absolute path to Relative
  =================================================================
  Info: Automatic replacement of the >absolute< path 
  to >relative< for the Pedestrian Tool positioning result files
  =================================================================

  Searching for >>  $SEARCH
  Replacing by  >>  $REPLACE

  Found in files:
  =================
EOF
SEARCH=$(dirname "$SCRIPT_PATH")"/"
SEARCH=$(dirname "$SCRIPT_PATH")/
directories=$(find $adresar*/ -type d)

for d in $directories
oldIFS="$IFS"
IFS=$'\n'

directories=$(find $adresar*/ -type d)

for d in $directories; do
    # do work ...
done

IFS="$oldIFS"

Context

StackExchange Code Review Q#101547, answer score: 4

Revisions (0)

No revisions yet.