New to scripting. looking for critiques / advice

[Plight]

Hello. Im new to shell scripting and am looking for any advice on the following code. The script performs the following:

1) downloads files into 2 locations from ftp server

2) verifies file exists in $app location (an application processes the file from there) before deleting it from the ftp

3) checks a local folder for a file, if file exists, it uploads it to the ftp

Any advice or critiquing is welcome!

# Copies down the files in two locations -
# App directory and tmporary directory for redunadancy.

ftp -n -i $1 <<EOF
user $USERNAME $PASSWORD
cd /Export
lcd $temp_dir
mget *.txt
lcd $app_dir
mget *.txt
bye
EOF


# Verifies File exists in app directory then deletes
# temp file and deletes from remote server


find $temp_dir/ -iname "*txt" -printf %f\\n | \
while read I; do
if [ -f "$app_dir"/"$I" ]; then
        echo ""$I" Allready Exists In Star Folder. Removing Temp File... "
        rm -f $temp_dir/"$I"
        echo "Deleting File From Remote Host.."
        ftp -n -i $1 <<EOF
        user $USERNAME $PASSWORD
        cd /Export
        mdelete *.txt
        bye
EOF
else
        echo "$I Does Not Exist. Copying File to Star Data Folder..."
        cp $temp_dir/"$I" $app_dir
fi
done;



# Checks for updates in local /Updates folder and sends to remote machine
# (Still needs to be based on machine name)

updates_dir="/updates"

if [ "$(ls -A $updates_dir)" ]; then
        echo "Update file found! Sending to remote host."
        ftp -n -i $1 <<EOF
        user $USERNAME $PASSWORD
        cd Updates
        mput $updates_dir/*
        quit
EOF
else
        echo "No updates to install"
fi

[Lakshmipathi]

Just my thought

rm -f $temp_dir/"$I"

Using rm -f is always a risky affair. So i would avoid applying rm on variable.
what if the variable is empty?
So I would suggest you to move to trash directory and then delete it.

mkdir ~/trash
mv $temp_dir/"$I" ~/trash
rm -rf ~/trash #we are not using any variable but exact path

[marafa]

for security reasons you should put the username and password for the ftp server in ~/.netrc

[unlimitedscolobb]

for security reasons you should put the username and password for the ftp server in ~/.netrc

And then chmod 0600 ~/.netrc so that nobody can see it.

Also, doing something like:

...
find $temp_dir/ -iname "*txt" -printf %f\\n | \
...

risks getting a successful match against files like hellotxt (note the absence of the dot). While not critical in itself, it makes the constraints looser and thus is a potential source of problems. I might be paranoid, but I'm trying to keep the tradition of specifying the dot