LinuxQuestions.org
Did you know LQ has a Linux Hardware Compatibility List?
Go Back   LinuxQuestions.org > Forums > Non-*NIX Forums > Programming
User Name
Password
Programming This forum is for all programming questions.
The question does not have to be directly related to Linux and any language is fair game.

Notices

Reply
 
Search this Thread
Old 10-02-2012, 05:12 AM   #1
jamesvjj
LQ Newbie
 
Registered: Oct 2012
Location: Philippines
Distribution: Ubuntu 12.04
Posts: 2

Rep: Reputation: Disabled
Looking for suggestions on bash Script


I wrote the following bash script. The goal of the script is to read in a file with a list of file names, then copy those files from one directory to another directory. The list of file names has 197 file names, the source directory has over 3000 plus files.

This worked for me, but I am looking for suggestions on how I can improve this script.

#!/bin/bash
sourcedir=~/Desktop/images
targetdir=~/Desktop/images_g1
file="./imagefilenames_g1.csv"
value=0
exec<$file
while read line
do
value=`expr $value + 1`;
echo $value;
image=`sed -n "$value"\p $file`;
echo "$image";
cp $sourcedir/"$image" $targetdir;
done

I am new to bash scripting and I'm not a programmer. I was thrilled I got this to work, and I'll be using it on and off to copy files for various different groups. Doing this got me interested in learning more about bash.

Thanks!
 
Old 10-02-2012, 05:21 AM   #2
pan64
Senior Member
 
Registered: Mar 2012
Location: Hungary
Distribution: debian i686 (solaris)
Posts: 4,658

Rep: Reputation: 1252Reputation: 1252Reputation: 1252Reputation: 1252Reputation: 1252Reputation: 1252Reputation: 1252Reputation: 1252Reputation: 1252
this looks like a one-liner in awk or perl or something like this in bash:
Code:
for image in `cat ./imagefilenames_g1.csv`
do
cp "$sourcedir/$image" $targetdir;
done
how do you use the loop variable line in your original script?
 
1 members found this post helpful.
Old 10-02-2012, 06:45 AM   #3
jamesvjj
LQ Newbie
 
Registered: Oct 2012
Location: Philippines
Distribution: Ubuntu 12.04
Posts: 2

Original Poster
Rep: Reputation: Disabled
I was looping through each line of the file and using the sed command. Too complicated, your solution is much better. Thanks!
 
Old 10-02-2012, 06:51 AM   #4
pan64
Senior Member
 
Registered: Mar 2012
Location: Hungary
Distribution: debian i686 (solaris)
Posts: 4,658

Rep: Reputation: 1252Reputation: 1252Reputation: 1252Reputation: 1252Reputation: 1252Reputation: 1252Reputation: 1252Reputation: 1252Reputation: 1252
while read line will read config file line by line. You could use it in cp, but you did not do that:
Code:
# ...
# same as before

while read image
do
    echo "$image";
    cp $sourcedir/"$image" $targetdir;
done
is ok too
 
1 members found this post helpful.
Old 10-04-2012, 04:11 AM   #5
David the H.
Bash Guru
 
Registered: Jun 2004
Location: Osaka, Japan
Distribution: Debian sid + kde 3.5 & 4.4
Posts: 6,823

Rep: Reputation: 1947Reputation: 1947Reputation: 1947Reputation: 1947Reputation: 1947Reputation: 1947Reputation: 1947Reputation: 1947Reputation: 1947Reputation: 1947Reputation: 1947
I'd like to add a few comments about the OP script:

1) Please use ***[code][/code] tags*** around your code and data, to preserve the original formatting and to improve readability. Do not use quote tags, bolding, colors, "start/end" lines, or other creative techniques.

2)
Code:
sourcedir=~/Desktop/images
"~" is an interactive alias. In a script it's usually better to use the built-in "$HOME" variable.

3)
Code:
exec<$file
a. QUOTE ALL OF YOUR VARIABLE SUBSTITUTIONS. You should never leave the quotes off a parameter expansion unless you explicitly want the resulting string to be word-split by the shell (globbing patterns are also expanded). This is a vitally important concept in scripting, so train yourself to do it correctly now. You can learn about the exceptions later.

http://mywiki.wooledge.org/Arguments
http://mywiki.wooledge.org/WordSplitting
http://mywiki.wooledge.org/Quotes

(Actually, this script is ok, since $file has been previously set to a safe value. But do it anyway to get into the habit! )

b. The line as written is completely unneeded. There's no need set up a fancy file descriptor in most scripts, and even then you'd probably use a different descriptor number.

4)
Code:
while read line...done
Again, instead of using an exec redirect to feed the while loop, just use 'done <"$file"' to redirect the file contents into it directly. It's cleaner and makes the code easier to comprehend.

5)
Code:
value=`expr $value + 1`;
a. $(..) is highly recommended over `..`

b. expr is wholly unnecessary in any modern shell. integer-based arithmetic is built-in.
Try a simple "(( value++ ))" instead.

6)
Code:
cp $sourcedir/"$image" $targetdir;
The quoting is all wrong here; you're leaving $sourcedir and $targetdir completely unprotected. The general rule of thumb is to simply quote the longest string possible. e.g. ...

Code:
cp "$sourcedir/$image" "$targetdir"
 
  


Reply


Thread Tools Search this Thread
Search this Thread:

Advanced Search

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is Off
HTML code is Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Variables and Mkvextract in a bash script and a good resource for bash help? gohmifune Linux - General 9 04-13-2011 08:37 AM
SSH connection from BASH script stops further BASH script commands tardis1 Linux - Newbie 3 12-06-2010 08:56 AM
[SOLVED] [BASH]first script, suggestions for improving! RaptorX Programming 9 08-29-2009 05:11 PM
[SOLVED] Using a long Bash command including single quotes and pipes in a Bash script antcore Linux - General 9 07-22-2009 11:10 AM
bash script suggestions for waiting for ssh -L iggymac Programming 4 03-22-2005 07:04 PM


All times are GMT -5. The time now is 09:16 PM.

Main Menu
My LQ
Write for LQ
LinuxQuestions.org is looking for people interested in writing Editorials, Articles, Reviews, and more. If you'd like to contribute content, let us know.
Main Menu
Syndicate
RSS1  Latest Threads
RSS1  LQ News
Twitter: @linuxquestions
identi.ca: @linuxquestions
Facebook: linuxquestions Google+: linuxquestions
Open Source Consulting | Domain Registration