help with a ksh script

reborg · May 6, 2007, 10:25pm

Absolutely, comments (in code) should be used only where needed. Well written code needs very few comments to explain it. Professional scripts will usually have little more than header and function comments.

Perderabo · May 7, 2007, 12:00am

csnewbie84:

Alrighty...here's what I got (with your help...hopefully I did this right...just learning this stuff 2 weeks ago)
#!/bin/ksh
pattern1=$1
pattern2=$2

if [ $# -ne 2 ]
then
print "Usage: $0 $pattern1 $pattern2"
exit
fi

for file in *
do
cp $3 /tmp/$3
sed -e 's/$pattern1/$pattern2/g'/tmp/$3 > $3
done
Does that look about right?

BTW In case you wondering why I using $3. $3 is the name of the file in which to make the substitution.

The code inside the for loop looks good after cfajohnson's changes are applied. But the for loop itself needs to go away. cfajohnson's code:
cp "$3" "/tmp/$3"
sed -e "s/$pattern1/$pattern2/g" "/tmp/$3" > "$3"
is good, but I did not see an explicit mention of eliminating that for loop. You do not want to perform the above two lines n times where n is the number of files in the directory. Then TinWalrus proposed alternate code that keeps the for loop. You need to nail this down. Should the script operate on one file? Or should it loop and operate on all files?

As for the && thing... I use && sometimes where I want the statements to be tightly coupled. A contrived example...
cd $MY_TEMP_DIRECTORY && rm -rf *

The alternate code:
if cd $MY_TEMP_DIRECTORY ; then
��rm -rf *
fi
scares me because I'm afraid the rm might get separated from the cd, maybe by inserting some debug code or something like that. Also another programmer might think that intent of the "if" statement is to simply check for the existence of of the directory. A lot of shell programmers think that an "if" statement only tests for something and they miss the fact that useful work is happening as well.

TinWalrus · May 7, 2007, 12:18am

professionally speaking, from a large shop perspective where there are windows and unix admins who cross train - documenting a script is of high importance. a person with limited unix experience (especially one coming from the windows world) will have a difficult time understanding a shell script. it is also of critical importance to use version control and a central repository, especially if the script is going to be used on multiple systems.

reborg · May 7, 2007, 10:54am

I guess we're way off topic by now, but anyway.

I can certainly understand your reasoning there. I have never come across this situation. I have always worked in environments where one or more people are assigned the task of maintaining a script, but then again I have worked mostly in R&D where the rules about touching code are possibly more restrictive. Having people not familiar with Unix touching the code just wouldn't happen ( unless a trainee under supervision ).

I guess I should clarify my position.

I have no objection to the use of && or || in an example like that given by Perderabo, where it is used to perform a compound operations. This should and does read as a single line of code.

I do however have a problem with something like this:
command && { command2; command3 } || { command4; command 5}

Which was what I originally commented on ( the use of braces )

I don't have a problem with commenting, I do have a problem with using comments to explain code which which should not need to be explained. If simple logic needs to be explained then the script (or the person reading it ) is in my opinion broken.

Using simple syntax usually means the less commenting is needed, after all an if statement is an if statement irrespective of the syntax of the language used. A person who cannot understand an if/else statement should not be maintaining the code, on the other hand I could forgive someone from having problem understanding the example above. The K.I.S.S. approach has long since proven it's effectiveness.

I agree completely, also in support of multi-track development this is essential, changes for one track may not be expected in another.

csnewbie84 · May 7, 2007, 6:00pm

First of all, Sorry for the late response...been a lonnnng weekend for me but I am learning a lot from the responses that I am getting. I just started with Unix programming for school and appreciate all of you helping a "newbie" out

Well the situation is that (just found out Sat. on what I was supposed to do) I have to have a loop but the filenames have to come from the command line and not from the loop (so I guess "foreach" loop is out the question) but I pretty sure that the sed command and either cp or mv command have to be invoked. What loop you think would work for this?

cfajohnson · May 7, 2007, 6:18pm

for file in "$@"
do
  ...