6
\$\begingroup\$

I wrote a bash script to move certain files within a group of directories with filenames that match certain conditions to another group of directories, but it is very cluttered. If possible, I want some help and tips regarding optimizing it:

#!/bin/bash

#Asks for user confirmation
echo "This will move all files in the School folder that are not text files to the SchoolOld folder"
echo
echo "P.S. It will move everything with a name timestamp less than or equal to today."
echo
echo -n "Press [Y] to move all of the schoolwork you've already done to a separate folder or [N] to cancel: "
old_stty_cfg=$(stty -g)
stty raw -echo
answer=$( while ! head -c 1 | grep -i '[ny]' ;do true ;done )
stty $old_stty_cfg
if echo "$answer" | grep -iq "^y" ;then
        sleep 1
else
    echo -e "\n"
        exit 1
fi


#Sets variables
mn=$(date +'%m')
dy=$(date +'%d')
mf=$((10#$mn))
df=$((10#$dy))
txt=$((0))


#Asks user how many days to go back
while [[ $count -gt 91 ]] 2> /dev/null || [[ $count -lt 1 ]] 2> /dev/null
do
    echo -e "\n"
    echo -n "How many days do you want to go back and save? (max. 90): "
    read -e count
    if [[ $count -gt 90 ]] 2> /dev/null || [[ $count -lt 1 ]] 2> /dev/null ; then
        echo -e "\n"
        echo "Please try again."
    fi
done

if ! [ $count -eq $count 2> /dev/null ]; then #Checks if entered value is an integer
    echo
    echo "Only integers are allowed. Exiting."
    exit 1
fi


#Asks for user confirmation
echo
echo "Do you also want to delete any text files with a timestamp less than or equal to today? [Y/N]"
old_stty_cfg=$(stty -g)
stty raw -echo
answer=$( while ! head -c 1 | grep -i '[ny]' ;do true ;done )
stty $old_stty_cfg
if echo "$answer" | grep -iq "^y" ;then
        ((txt=1))
else
        ((txt=0))
fi



#Moves files
while [[ $df -gt 0 ]] && [[ $count -gt 0 ]]; do
    echo "The curent month in \$mf is $mf"
echo "The current day in \$df is $df"

        find ./School/Art -iname "*$df*-*$mf*.*" ! '(' -iname "*.txt" ')' -exec mv "{}" -t "./SchoolOld/Art" \;
        find ./School/Comp -iname "*$df*-*$mf*.*" ! '(' -iname "*.txt" ')' -exec mv "{}" -t "./SchoolOld/Comp" \;
        find ./School/Cont -iname "*$df*-*$mf*.*" ! '(' -iname "*.txt" ')' -exec mv "{}" -t "./SchoolOld/Cont" \;
        find ./School/Eng -iname "*$df*-*$mf*.*" ! '(' -iname "*.txt" ')' -exec mv "{}" -t "./SchoolOld/Eng" \;
        find ./School/Esp -iname "*$df*-*$mf*.*" ! '(' -iname "*.txt" ')' -exec mv "{}" -t "./SchoolOld/Esp" \;
        find ./School/Geo -iname "*$df*-*$mf*.*" ! '(' -iname "*.txt" ')' -exec mv "{}" -t "./SchoolOld/Geo" \;
        find ./School/Hist -iname "*$df*-*$mf*.*" ! '(' -iname "*.txt" ')' -exec mv "{}" -t "./SchoolOld/Hist" \;
        find ./School/LPT -iname "*$df*-*$mf*.*" ! '(' -iname "*.txt" ')' -exec mv "{}" -t "./SchoolOld/LPT" \;
        find ./School/Math -iname "*$df*-*$mf*.*" ! '(' -iname "*.txt" ')' -exec mv "{}" -t "./SchoolOld/Math" \;
        find ./School/P.E -iname "*$df*-*$mf*.*" ! '(' -iname "*.txt" ')' -exec mv "{}" -t "./SchoolOld/P.E" \;
    find ./School/Port -iname "*$df*-*$mf*.*" ! '(' -iname "*.txt" ')' -exec mv "{}" -t "./SchoolOld/Port" \;
        find ./School/Sci -iname "*$df*-*$mf*.*" ! '(' -iname "*.txt" ')' -exec mv "{}" -t "./SchoolOld/Sci" \;


    if [[ $txt -eq 1 ]]; then
        find ./School -iname "*$df*-*$mf*.txt" -exec rm -i "{}" \;  #Deletes the text files
    fi


        if [[ $df -eq 1 ]]; then #Sets month values
                if [[ $mf -eq 02 ]]; then
            ((df=31))
                fi

                if [[ $mf -eq 4 ]] || [[ $mf -eq 6 ]] || [[ $mf -eq 9 ]] || [[ $mf -eq 11 ]]; then
            ((df=30))
                else
            ((df=31))
                fi

                if [[ $mf -eq 1 ]] && [[ $df -eq 1 ]]; then
                        echo "Cannot return back more than a year."
                        exit 1
                fi
        ((mf=mf-1))
        echo "the month now is $mf"
    fi
    ((df=df-1))
        ((count=count-1))
done
\$\endgroup\$
2
\$\begingroup\$

Your code doesn't contain sample data, so any following code adjustments are made on just what I read from your code.

answer=$( while ! head -c 1 | grep -i '[ny]' ;do true ;done )
stty $old_stty_cfg
if echo "$answer" | grep -iq "^y" ;then
        sleep 1
else
    echo -e "\n"
        exit 1
fi

Could be written as:

read -p "Are you sure? " -n 1 -r
if ! [[ $REPLY =~ ^[Yy]$ ]] ; then
  exit 1   
fi

Not only is this way easier to read, but requires so much less hacking of the tty sessions to do so. Also, I'm of the opinion, if they answer anything but "Y" then exit the script immediately. In your case, YN is a funny mistype, but is a valid input which would cause your script to continue.

if [[ $count -gt 90 ]] 2> /dev/null || [[ $count -lt 1 ]] 2> /dev/null

Should be written as:

if [[ ( $count -gt 90 ) && ( $count -lt 1 ) ]] ; then

This is one statement which represent multiple conditions. Again, this is sanitizing your variables and only using what you expect to happen.

I do not know why you are redirecting stderr to /dev/null and would be interested to see what bash flags here as errors which would be crucial to critiquing your code.

   find ./School/Art -iname "*$df*-*$mf*.*" ! '(' -iname "*.txt" ')' -exec mv "{}" -t "./SchoolOld/Art" \;
    find ./School/Comp -iname "*$df*-*$mf*.*" ! '(' -iname "*.txt" ')' -exec mv "{}" -t "./SchoolOld/Comp" \;
    find ./School/Cont -iname "*$df*-*$mf*.*" ! '(' -iname "*.txt" ')' -exec mv "{}" -t "./SchoolOld/Cont" \;

Whenever you do something multiple times, consider using a for loop.

for DIRECTORY in Art Comp Cont Eng Esp Geo Hist LPT; do 
    find ./School/${DIRECTORY} -iname "*$df*-*$mf*.*" ! '(' -iname "*.txt" ')' -exec mv "{}" -t "./SchoolOld/${DIRECTORY}" \;
done
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.