This code generally works exactly as it is supposed to. I'm learning bash I'm looking for some constructive help that could help me improve my work.

#!/bin/bash



usage()
{
    echo "Program for uncompressing  tar, gzip, rar archives"
    echo "Allowed parametres: --help, --file, --input, --directory"
    echo "Examples:"
    echo "./uncompress.sh --file firstFile secondFile"
    echo "./uncompress.sh --input inputFile.txt"
    echo "./uncompress.sh --directory /home/"

}


uncompress_file()
{
    if [ -f $1 ]
    then
        temp1=`file $1 | cut -d: -f2 | grep tar`

            if [ "$temp1" != "" ]
                then
                    echo    "$1 is a tar archive, uncompressing..."
                    tar xf $1 > /dev/null
            fi

        temp1=`file $1 | cut -d: -f2 | grep RAR`

            if [ "$temp1" != "" ]
                then
                    echo    "$1 is a rar archive, uncompressing..."
                    unrar e $1 > /dev/null
            fi  

        temp1=`file $1 | cut -d: -f2 | grep gzip`

            if [ "$temp1" != "" ]
                then
                    echo    "$1 is a gzip archive, uncompressing..."
                    tar xzf $1 > /dev/null

            fi


    else
    echo    "$1 is not a valid file!"
    fi
}

uncompress_dir()
{
    for file in "$1"/*
        do
            if [ -f $file ]
            then
            uncompress_file $file
            fi


            if [ -d $file ]
            then
            uncompress_dir $file
            fi


        done

}







if [ "$1" = "--help" ]
    then
    usage
    exit

fi

if test $# -lt 2
    then echo "What about parametres?" 1>&2
    exit 1
fi



param=$1;
shift;





if [ "$param" = "--file" ]
    then

        while [ $# -ne 0 ]
        do
            uncompress_file $1
            shift
        done
    exit
fi

if [ "$param" = "--input" ]
    then
        if [ -f $1 ]
            then
                while read line
                do
                    uncompress_file $line

                done < $1       
            else
                echo "$1 is not a valid file"
        fi
    exit    
fi


if [ "$param" = "--directory" ]
    then
        if [ -d $1 ]
            then
                uncompress_dir $1
            else
            echo    "$1 is not a valid directory"
        fi
    exit
fi


echo "wrong parametres!";
up vote 4 down vote accepted
  1. Don't use cut and grep, use parameter substitution and a case statement over the file extension to decide the file type (or use the file command if you want to be really clever, but I recommend the former actually)
  2. Don't use [ "${foo}" != "" ], use [ -n "${foo}" ]
  3. Don't mix test and [, just stick to [ unless you want to write code that looks autoconf generated
  4. Quote all your variables, especially the ones containing filenames to account for spaces, e.g. tar xzf "$1" > /dev/null
  5. Don't parse command line options yourself, use getopts (or getopt, in case you really, really want GNU-style options)
  6. Improve your indentation
  • But file extension doesn't always match content of the file. How could You deal with it then? This is why I decided to extract desired information with file. – Grzegorz Piwowarek May 10 '13 at 21:13
  • My thought pattern is that if a file has the "wrong" extension, in many cases you will want to know about that and not silently continue. The decompressor should error with an appropriate message anyway. Or you could e.g. implement a -g (for "guess") switch to use file instead of the file extension. But as I said, this is a matter of preference; if you want to use file, go ahead ;-) As an addition source of inspiration you might want to check the unpack function that Gentoo's portage uses here. – Adrian Frühwirth May 10 '13 at 21:30

grep has a non-zero exit status if it finds no matches. So code like this

temp1=`file $1 | cut -d: -f2 | grep tar`
if [ "$temp1" != "" ]
then
    echo    "$1 is a tar archive, uncompressing..."
    tar xf $1 > /dev/null
fi

can be more succinctly written as

# -q causes grep to output nothing; we don't really care what it finds,
# just if it finds *something*
if file $1 | cut -d: -f2 | grep -q tar
then
    echo "$1 is a tar archive, uncompressing..."
    tar xf "$1" > /dev/null
fi

Always quote parameter expansions, as I did above for the call to tar, just in case the value contains whitespace. It's almost never wrong to do so, and you never know when it might be absolutely necessary.

Your Answer

 
discard

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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