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.

Don't parse ls. Just use for file in *, and even that is subject to problems. — DopeGhoti
– DopeGhoti, Commented Feb 8, 2017 at 18:02

AndyB · Accepted Answer · 2017-02-08 20:51:54Z

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
– DopeGhoti, Commented Feb 8, 2017 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
– Sergiy Kolodyazhnyy, Commented Feb 8, 2017 at 18:16
Thank you so much! To be honest, using a counter did not come to mind! — Spencer Wattamaniuk
– Spencer Wattamaniuk, Commented Feb 8, 2017 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
– Sergiy Kolodyazhnyy, Commented Feb 8, 2017 at 18:24

Stack Exchange Network

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

1 Answer 1

You must log in to answer this question.

Linked

Hot Network Questions

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

1 Answer 1

You must log in to answer this question.

Linked

Related

Hot Network Questions