Checking all Files in a `ls` for loop and displaying a message based on a condition

Question

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.

requisite reading: http://unix.stackexchange.com/q/128985/117549 — Jeff Schaller, Feb 08 '17 at 17:54
Don't parse ls. Just use for file in *, and even that is subject to problems. — DopeGhoti, Feb 08 '17 at 18:02

score 3 · Accepted Answer · edited Feb 08 '17 at 20:51

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
Don't parse ls. Use shell globbing instead:
```
for file in *
```
Quote all your variables:
```
if [ ! -s "$file" ]
```
exit 55 is not a very common type to represent errors. Usually people use exit 1.
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

An exit code of 55 is perfectly fine. I routinely use different exit codes to alert on specific error conditions. — DopeGhoti, Feb 08 '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 08 '17 at 18:16
Thank you so much! To be honest, using a counter did not come to mind! — Spencer Wattamaniuk, Feb 08 '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 08 '17 at 18:24
The reason I am using exit 55 is because the specs of the script call for it. — Spencer Wattamaniuk, Feb 08 '17 at 18:27
@SpencerWattamaniuk Let me guess - homework, right ? Just keep in mind that just because something is required in homework doesn't necessarily mean it's the Right Way ® in the real world :) In any case, good luck, enjoy Linux, keep on learning ! — Sergiy Kolodyazhnyy, Feb 08 '17 at 18:29

Checking all Files in a `ls` for loop and displaying a message based on a condition

1 Answers1