2
\$\begingroup\$

I have been working on a bash script for a while now to organize my movie files. I can do it by hand, but at the time I was taking a bash scripting class and thought I would like to use it to create this project. I don't know if there is a better way of doing it or not, but I would like you to take a look at my script and give me feedback on it. I believe it works, but I haven't put it through the paces yet.

#!/bin/bash

MovieList="points to .txt file with name of files"
DirList="points to a .txt file with the name to use to create the directory"
dest_Dir="where to put the files "
src_Dir="where to look for the files"
CreateLogFile="log file of directory created"
MoveLogFile="log file of move files"

# Will read in a list of movies and make the directories for them

createDirectory()
{
    while read -r line
    do
    if [ ! -d $dest_Dir"$line" ]
    then
            mkdir -v $dest_Dir"$line" >> $CreateLogFile
    else
            :
    fi
done < "$DirList"
}
# end createDirectory

# After directories have been created will go and find those movies and move
# them to their proper directory

movieMovies()
{
    while read -r line
do
    find $src_Dir  -type f \( -iname "$line" ! -iname ".*" \) | xargs -I '{}' mv -v {} $dest_Dir"$line" >> $MoveLogFile
done < "$MovieList"
}
# end of moveMovies

# Time to tidy up by going back and seeing if there is any empty directory
# and if so removing said directory

DirectoryEmpty()
{
while read -r line
do
    if [ $(ls -A $dest_Dir"$line" | wc -l) -eq 0 ]
    then
        rmdir $dest_Dir"$line" 
    else
        :
    fi
done < "$DirList"
}
# end of DirectoryEmpty

# ---------------------------------Main---------------------------------------
createDirectory
moveMovies
DirectoryEmpty
# ----------------------------------End---------------------------------------
\$\endgroup\$

migrated from stackoverflow.com Jun 30 '13 at 23:24

This question came from our site for professional and enthusiast programmers.

  • \$\begingroup\$ If you don’t know of a problem, try Code Review instead. \$\endgroup\$ – Ry- Jun 30 '13 at 23:18
1
\$\begingroup\$

errors

  • typo: function named movieMovie, invoked as moveMovie

potential errors

  1. use of $dest_Dir"$line"
    • you expect dest_Dir to end with a slash, but that's not validated anywhere
    • use "$dest_Dir/$line" instead (it's OK to have multiple slashes)
  2. don't use ls to verify the presence/absence of files:

    removeEmptyDirectory() {
        dir="$1"
        shopt -s globstar nullglob dotglob
        files=("$dir"/*)  # an array holding all the filenames
        if (( ${#files[@]} == 0 )); then
            # the directory is empty
            rmdir "$dir"
        else
            echo "warning: directory not empty: $dir" >&2
        fi
    }
    

    Even better, use find:

    removeEmptyDirectories() {
        while read -r line; do
            find "$dest_Dir/$line" -depth 0 -empty -exec rmdir '{}' \;
        done < "$DirList"
    }
    
  3. Double quote all your variables, everywhere, unless you have a specific reason not to.

style

  1. clean up your indentation. It is idiomatic to put then and do at the end of the same line as if and while (see removeEmptyDirectories() example above).
  2. use a consistent naming convention
    • variables DirList, src_Dir -- use camelcase or underscores, but both?
  3. don't need the else : branch
  4. "line" is not a descriptive variable name.
  5. use find -exec instead of piping to xargs.
\$\endgroup\$

Your Answer

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