topic Re: Improving an existing script in Operating System - Linux

Improving an existing script

Andrew Kaplan — Thu, 22 Oct 2009 21:31:07 GMT

Hi there --

I have a script running on one of our workstations that converts .ps files to .pdf versions using the ps2pdf binary. The text of the script is shown below:

##############################################
#!/bin/bash

# The purpose of this script is to go to the directory in question,
# and convert all postscript, .ps, files located there to .pdf files.

# Provide a list of the cases directory for the user.
cd /home/rsa/data/xknife_data/cases

echo CONVERSION OF POSTSCRIPT, .ps, FILES TO ADOBE ACROBAT, .pdf, FORMAT.
echo
echo
echo The patients in the cases folder will now be listed for your convenience.
echo
echo It is important that you make note of the exact spelling of the directory
echo in question in order for this process to work.
echo
echo
read -p "Press the enter key to continue..."

ls -1 | more
echo
echo
echo Please enter the directory that is the object of this exercise.
echo If you make a mistake here, hit CTRL-C to exit from the script,
echo and start over.
echo
read directory
echo Is $directory the patient in question?

if [ -e $directory ]
then
echo "Patient directory found."
else
echo "Patient directory NOT found."
fi

# Change to the directory in question
cd /home/rsa/data/xknife_data/cases/$directory

# Copy the .ps files to a local directory
cp -pr *.ps /tmp/ps2pdf

# Change to the temporary directory
cd /tmp/ps2pdf

# List the files that were copied over the temporary directory
ls -l

# Convert the files in the temporary directory to .pdf format.
for i in *.ps;
do ps2pdf "$i";
done

# Copy the .pdf files back to the original location
cp -pr *.pdf /home/rsa/data/xknife_data/cases/$directory

# Remove the files from the temporary directory
cd /tmp/ps2pdf
rm -rf *.ps
rm -rf *.pdf

echo The conversion of the postscript files to acrobat format is complete.
echo Use an ftp client application to retrieve the files from the patient directory.
##############################################

I have been asked to add the following improvement to the script:

Prompt the user to answer yes or no as to the name entered is spelled correctly. If so, the script continues, otherwise prompt the user to reenter the name of the directory again.

I am not familar with the correct syntax to enable the item that I listed, so I was hoping someone could lend a hand here. Thanks.

Re: Improving an existing script

Steven Schweda — Thu, 22 Oct 2009 22:59:05 GMT

Have you considered the advantages of
learning some basic computer programming
somewhere?

> if [ -e $directory ]
> then
> echo "Patient directory found."
> else
> echo "Patient directory NOT found."
> fi

So, if "$directory" exists or not, this
script continues on, and uses that name,
no matter how hopeless the task might be with
the name of a nonexistent directory.

It might be more helpful to go back and ask
again when the user supplies a bad answer.
There are many ways to do this, including
"while", "if", and some of their relatives
("goto", ...).

man bash

> ls -1 | more

You might tell the user how to abort the
listing when the desired name is seen.

With a little more effort, one might present
to the user a screen-full of names with
(shorter, easier-to-type) numbers. For
example:

1. Aardvark
2. Kaplan
3. Smith
4. Zamboni

and then take a number as input. (You could
then display the associated name, and ask the
user for confirmation before doing the work.)

The world is filled with shell scripts which
do useful things in clever ways. You might
try to find some, and steal their secrets.

Or buy a book. Or take a class.

Or, I suppose, wait for someone here to offer
to do your job for you.

Re: Improving an existing script

Steven Schweda — Fri, 23 Oct 2009 05:05:34 GMT

I must have too much time on my hands.

> # Copy the .ps files to a local directory

Why?

> # Copy the .pdf files back to the original location

Again, why? Aren't they still there?

> # Copy the .pdf files back to the original location

Can't "ps2pdf" create them there?

> # Remove the files from the temporary directory

You don't need to delete the ".ps" files if
you don't do the unnecessary copying. You
don't need to delete the ".pdf" files if you
create them in the right place to begin with
(or if you "mv" them instead of copying
them, but that's still extra work).

Miscellaneous complaints:

> if [ -e $directory ]

> cp -pr *.pdf /home/rsa/data/xknife_data/cases/$directory

> [...]

I hope that there are no spaces in any of
these directory names.

Using "echo" with unquoted text can cause
unpleasant things to happen. For example,
compare these:
echo 'a > b'
echo a > b

For a simple example script which shows a
couple of basic things, consider this:

bash$ cat example.sh
#!/bin/sh

cat < Here's how to display some text
without using "echo" commands.

It even does multiple spaces
and special characters, like
( ) < > | \ ; $ " '
(without using quotation marks or
apostrophes).

Begin input section...
EOD

ans_ok=0
while [ ${ans_ok} -eq 0 ] ; do
echo -n " Type \"fred\" (or don't -- \"quit\" to quit): "
read line
if [ "${line}" = 'fred' ] ; then
ans_ok=1
else
l1=` echo "${line}" | cut -c 1 `
if [ "${l1}" = 'q' -o "${l1}" = 'Q' ] ; then
ans_ok=-1
fi
fi
if [ ${ans_ok} -eq 0 ] ; then
echo " Invalid response: >${line}<. Try again..."
fi
done

echo " ans_ok = ${ans_ok}, line: >${line}<."

It's probably not ideal, but it does a few
things properly.

Re: Improving an existing script

Steven Schweda — Fri, 23 Oct 2009 14:29:53 GMT

A particularly common "echo" hazard:

bash$ cat echo1.sh
#!/bin/sh

echo "This'll work."
echo This won't work.
echo "This would've worked."

Unless you're a particular member of the crew
of the starship Enterprise, unquoted "echo"
text can cause problems, even for text which
is pretty ordinary (for most folks).

Re: Improving an existing script

Andrew Kaplan — Fri, 23 Oct 2009 15:15:58 GMT

I am going through the process of implementing your suggestions, and this posting is a response to the questions of your previous posting:

The reason I copy the ps files from the original directory to the temporary one is due to the original location being an NFS mount and the conversion of the ps files over the network to that location was taking prohibitively long to complete. Copying the files to a local directory, running the ps2pdf command there, and then copying them back proved to be better in regards to performance. The removing of the files from the temporary location is simply a house-keeping measure.

The directories themselves do not have any spaces within their names, and that is due to the application being used that creates them.

I will double-check my syntax with the echo statements, and accordingly modify them.

Thank-you for your help.

Re: Improving an existing script

Steven Schweda — Fri, 23 Oct 2009 18:47:00 GMT

> [...] conversion of the ps files over the
> network to that location was taking
> prohibitively long [...]

Really? You measured this carefully?
Including the copy operations? Both ends?

Knowing nothing about your "ps2pdf" program,
I can imagine that it might make multiple
passes through the input file, so, if your
network really is a bottleneck, then copying
the input file (once) to a local work file
might be helpful, but it's harder to imagine
that it writes the output file more than
once, so it's less obvious (to me) that
writing to a local file and then copying that
over the network would buy you anything.

> The removing of the files from the
> temporary location is simply a
> house-keeping measure.

If you actually need temporary files, then
deleting them when you're done is good. I
apparently got confused between ps and pdf,
and thought that you were copying the ps
files back to the source, not only the pdf
files. (Probably looking at the wrong "cp"
command. Trust no one, I always say.)

> The directories themselves do not have any
> spaces within their names, [...]

Still, quotation marks are pretty cheap, and
your script might live longer than the
current conditions. Also, writing robust
code can become a habit, but that generally
requires doing it.

Re: Improving an existing script

Andrew Kaplan — Fri, 23 Oct 2009 19:19:30 GMT

I did some further testing, and the code shown below is what I inserted into the script:

##########################################
read -p "Is the directory name that was entered spelled correctly "
if [ $REPLY = "no" ]; then
echo Reenter the patient directory name.
exit
fi
##########################################

This addition appears to work as far as the users are concerned.

Regarding your question about testing it at both ends: The test that I did was as follows, a patient directory was chosen, and the ps2pdf command was first run over the NFS mount. The calculated time for completion was approximately fifteen minutes. The generated pdf files were then deleted, and the same command was run after the .ps files were copied to a local directory, converted to pdf format, and then copied back to the source. The approximate time for completion was about five minutes.

Re: Improving an existing script

Steven Schweda — Fri, 23 Oct 2009 21:47:29 GMT

> The test that I did was as follows [...]

And what happens if you make a local copy of
the ".ps" file, but create the output file
directly on the remote file system?

Or the other way around?

And those with-local-copy times include the
copying?

> read -p "Is the directory name that was entered spelled correctly "
> if [ $REPLY = "no" ]; then

I'm sure glad that I don't need to use this
thing. In my neighborhood, people would get
highly annoyed in they were compelled to type
"no", not "n", "N", "No", "nO", "NO", "NO!",
or so on. What happens if the user spells
"no" incorrectly? Does it make more sense to
test for "yes" or "no"? (If you think that
there's no difference, then think again.)
What should happen if the user responds to a
yes-no question with, say, "Fred"?

In most places, questions like this have a
default answer, so the prompt might look
like, say:
Question (Y/N [Y])?
or
Question (Y/n)?
and the program would interpret a null
response as "Yes".

> exit

What, asking again is too complex? Sheesh.

Did you try running that example script I
offered? Did you look at what it does?

Your users (or management) must be
exceptionally tolerant.