2

I created a script that checks if a certain directory exists; if not, then it is created. Then a for loop is used to run through all regular, non-hidden files in the current dir. If a file is empty, then prompt if the user would like it moved. Where I'm stuck is that the script, after running through the files, needs to check if all files are empty, and then displays a message saying so. Here's what I have.

#!/bin/bash
if [ -d Empty_Files ]
then
    echo "This file directory exists already. Move on"
else
    mkdir Empty_Files
    echo "I created the Empty_Files directory since it didn't exist before"
fi

for file in `ls`
do
 if [ ! -s $file ]
  then
  echo "Would you like" $file "moved to Empty_Files? It's empty! Enter Yes or No"
 read userinput
   if [ $userinput = "Yes" ]
    then 
    mv $file ./Empty_Files
   fi
 if [ $userinput = "No" ]
  then
    echo "Ok, I will not move it!"
 fi
 fi
done
 if [ -s $file ]
   then
     echo "There are no empty files!"
     exit 55
fi

As you can see, my if statement at the end doesn't work fully as intended.

3
  1. Don't use backquotes for command-substitutions(around ls ). It's not very readable and has issues. If you have to use output of another command, use $(command args1 args2) form instead

  2. Don't parse ls. Use shell globbing instead:

    for file in *
    
  3. Quote all your variables:

    if [ ! -s "$file" ]
    
  4. exit 55 is not a very common type to represent errors. Usually people use exit 1.

  5. Indent your code so that it's clear what each part is doing and where each if statement starts/ends, where each loop starts , ends.

Here's your fixed script

#!/bin/bash

if [ -d Empty_Files ]
then
    echo "This file directory exists already. Move on"
else
    mkdir Empty_Files
    echo "I created the Empty_Files directory since it didn't exist before"
fi

count=0
for file in *
do
    if [ ! -s "$file" ]
    then
        count=$(( $count+1  ))
        echo "Would you like" $file "moved to Empty_Files? It's empty! Enter Yes or No"
        read userinput

        if [ "$userinput" = "Yes" ]
        then 
            mv "$file" ./Empty_Files
        fi

        if [ "$userinput" = "No" ]
        then
            echo "Ok, I will not move it!"
        fi
fi
done

# quoting here not necessary because it's integer
if [ $count -eq 0  ];
then
     echo "There are no empty files!"
     exit 1
fi

Test run

[2821][TESTDIR][11:14]:
$ tree
.
├── Empty_Files
└── move_empty.sh

1 directory, 1 file

[2821][TESTDIR][11:14]:
$ ./move_empty.sh                                                                                     
This file directory exists already. Move on
There are no empty files!

[2821][TESTDIR][11:14]:
$ touch empty1 empty2

[2821][TESTDIR][11:14]:
$ ./move_empty.sh                                                                                     
This file directory exists already. Move on
Would you like empty1 moved to Empty_Files? It's empty! Enter Yes or No
Yes
Would you like empty2 moved to Empty_Files? It's empty! Enter Yes or No
No
Ok, I will not move it!

[2821][TESTDIR][11:14]:
$ tree
.
├── empty2
├── Empty_Files
│   └── empty1
└── move_empty.sh

1 directory, 3 files
| improve this answer | |
  • An exit code of 55 is perfectly fine. I routinely use different exit codes to alert on specific error conditions. – DopeGhoti Feb 8 '17 at 18:13
  • @DopeGhoti it's fine yet not frequently used. Most programs have exit 1 on error. It's just a suggestion, not a rule of thumb, of course, but if I'd be slightly pissed if my colleague wrote script that doesn't behave in a manner most scripts do. Or at least that has to be documented somewhere – Sergiy Kolodyazhnyy Feb 8 '17 at 18:16
  • Thank you so much! To be honest, using a counter did not come to mind! – Spencer Wattamaniuk Feb 8 '17 at 18:22
  • @DopeGhoti And what's you point ? The page says exactly what I said: However, many scripts use an exit 1 as a general bailout-upon-error. If you think I'm saying "using 55 is not allowed" , that's not what I'm saying. There has to be good reason for using exit 55 , which has to be documented. Feel free to ask other users about the opinion on this – Sergiy Kolodyazhnyy Feb 8 '17 at 18:24

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.