Can you give me some ideas on how to improve script?

Hi everyone! Hope you’re all having a nice life! :slight_smile:

So, I took over practicing/learning bash scripting (once again :D). I’m not very good at it and my knowledge is very basic. Last year I wrote a script to practice the use of if statements if something_happens then do this else do this what the script does/is about: cd to a dir, list files by extensions, if files with certain .ext exist, report they do, if they don’t, report so as well. I used ls Can you take a look at it and tell me how and where to improve? I’m sure there may be several ways to do it I just can’t figure it out by myself. FWIW, I’ve been reading and trying things but something’s always wrong. Here it is:

#!/bin/bash
#listar archivos en disco USB, Seagate.
#Creador: Moltke
#Octubre 2019
#El proposito principal es aprender a usar condicionales en un script.
#Algunas variables que hacen el script portable y reusable
Seagate="/media/moltke/Seagate"
linea_blanca="\e[1;37m ++++++++++++++++++++++++\e[0m"
echo -e “$linea_blanca”
if ( cd $Seagate && ls *.torrent &> /dev/null ); then
echo -e “\e[1;34m Archivos .torrent en Seagate:\e[0m”
else
echo -e “\e[1;31m Ningún archivo .torrent en Seagate!\e[0m”
fi

echo -e “$linea_blanca”
if ( cd $Seagate && ls *.mp3 &> /dev/null ); then
echo -e “\e[1;34m Archivos .mp3 en Seagate:\e[0m”
else
echo -e “\e[1;31m Ningún archivo .mp3 en Seagate!\e[0m”
fi

echo -e “$linea_blanca”
if ( cd $Seagate && ls *.txt &> /dev/null ); then
echo -e “\e[1;34m Archivos .txt en Seagate:\e[0m”
else
echo -e “\e[1;31m Ningún archivo .txt en Seagate!\e[0m”
fi

echo -e “$linea_blanca”
if ( cd $Seagate && ls *.png &> /dev/null ); then
echo -e “\e[1;34m Archivos de imagen en Seagate:\e[0m”
else
echo -e “\e[1;31m Ningun archivo de imagen en Seagate!\e[0m”
fi

echo -e “$linea_blanca”
if ( cd $Seagate && ls .m?? &> /dev/null ); then
echo -e “\e[1;34m Archivos de video en Seagate:\e[0m”
else
echo -e “\e[1;31m Ningún archivo de video en Seagate!\e[0m”
fi

echo -e “$linea_blanca”
if ( cd $Seagate && ls *.pdf &> /dev/null ); then
echo -e “\e[1;34m Archivos pdf en Seagate:\e[0m”
else
echo -e “\e[1;31m Ningún archivo pdf en Seagate!\e[0m”
fi

echo -e “$linea_blanca”
if ( cd $Seagate && ls *.sh &> /dev/null ); then
echo -e “\e[1;34m Archivos de script en Seagate:\e[0m”
else
echo -e “\e[1;31m Ningún archivo de script en Seagate!\e[0m”
fi

echo -e “$linea_blanca”
if ( cd $Seagate && ls *.iso &> /dev/null ); then
echo -e “\e[1;34m Archivos iso en Seagate:\e[0m”
else
echo -e “\e[1;31m Ningún archivo iso en Seagate!\e[0m”
fi

echo -e “$linea_blanca”
if ( cd $Seagate && ls *.zip &> /dev/null ); then
echo -e “\e[1;34m Archivos zip en Seagate:\e[0m”
else
echo -e “\e[1;31m Ningún archivo zip en Seagate!\e[0m”
fi

echo -e “$linea_blanca”
if ( cd $Seagate && ls *.aria2 &> /dev/null ); then
echo -e “\e[1;34m Archivos de aria2 en Seagate:\e[0m”
else
echo -e “\e[1;31m Ningún archivo de aria2 en Seagate!\e[0m”
fi

echo -e “$linea_blanca”
if ( cd $Seagate && ls *.srt &> /dev/null ); then
echo -e “\e[1;34m Archivos de subtitulos en Seagate:\e[0m”
else
echo -e “\e[1;31m Ningún archivo de subtitulos en Seagate!\e[0m”
fi

Thanks in advance for all your answers! :slight_smile:

NOTE: My native language’s Spanish so that’s what I use in my scripts for comments and messages so here’s what the lines in that language above say:

    • $linea_blanca = $white_line
    • Archivos .ext en Seagate = Files with .ext in Seagate
    • Ningún archivo con .ext en Seagate! = No files with .ext in Seagate!
    • listar archivos en disco USB, Seagate = list files in USB disk, Seagate.
    • Creador: Moltke = Creator: Moltke
    • Octubre 2019 = October 2019
    • El proposito principal es aprender a usar condicionales en un script = the main purpose is to learn how to use conditional statements in a script.
    • Algunas variables que hacen el script portable y reutilizable = some variables to make the script portable and reusable.

HINT: What I’m asking for is, if possible, something like “you can do this keyword see here some_link” I’m not asking for specific hacks, though they’re welcome :slight_smile: but rather that you point me in the right direction. Hope this makes sense to you and sorry in advance if it doesn’t.

I see a lot of repetitiveness which could be eliminated using functions, parameters and arguments:

https://stackoverflow.com/questions/6212219/passing-parameters-to-a-bash-function

1 Like

Thanks for your reply and the link. Will read it and see what I can get from it. Yeah, I’m well aware of the redundancy on the code but I can’t figure out how to make it simpler, better. I tried creating an array but failed, guess need to read more about it. Thing’s I can’t find a way to solve how to place/assign the ls command output to a variable so I don’t have to repeat it every time, I’d like to do something like somefunction/variabe="ls*.{extensions to list}" I just don’t know the right way to do that, been reading a few posts, blogs, here https://wiki.bash-hackers.org/start here https://tldp.org/LDP/Bash-Beginners-Guide/html/index.html a couple of books I have here as well as some other tutorials on the web.

I believe parsing the output of ls is frowned upon as it isn’t robust when dealing with spaces and such. You’ll find a lot of discussion of that particular point. Using find and looping might be a better shout if it’s complex (or the -exec flag is quite handy for simpler quieries).

Thanks for your reply. Not sure what you mean with "

ls is frowned upon

Well, the thing with using find is that it searchs for files recursively and I’d like to avoid that since that is a 1TB drive with lots of files, thus the output will be extemely long, dificult to read and navigate.

I’d advise you to look it up as it’ll be explained a lot better than I am capable of. You can add something like -prune or -maxdepth to find to reduce it’s wasted recursion or you can only pass directories of interest. Personally for something more complex than this I’d avoid bash entirely though that’s coloured a lot by the fact I don’t often make use of it and therefore have a limited knowledge of its use.

If you’re determined to write it so as to practice in bash you could consider performing the task in a language you know well and then porting it over. At least then you’ll have an idea of exactly what kind of tools you’ll want to be using. After that it’s just reading up on how to make use of them such as the functions link provided earlier.

Yes, you’re right. I’d completely forgotten find does have those options, after all, I don’t use it that much either.

Actually, I’m not very knowledgeable/skilled at or in any particular language. I use bits of code from here and there and try to learn by examples. I’m no coder or developer whatsoever. I’m just a regular user who wants to learn some coding so I can do some basic stuff. I started learning python a while ago too, here in codecademy but got stuck at some point and haven’t taken over it ever since. I also started learning html a while ago but got stuck at some point too, coincidentaly, both times when reached the variables topic, I have really a hard time trying to understand that, I think’s because the examples available seem to be made for sysadmins or intermediate-advanced users in mind but not end/regular desktop users. I use Linux by the way, Debian Buster but have another couple of systems installed too: Debian Bullseye (testing) and Arch Linux. Anyway, Thanks for your reply. I’ll read on how to properly use find and see what I can get from it. :slight_smile:

1 Like