Help with refining script


 
Thread Tools Search this Thread
Top Forums Shell Programming and Scripting Help with refining script
# 1  
Old 01-01-2017
Ubuntu Help with refining script

Happy new year all!

I would like to ask for some assistance refining the script I have put together below. I am a noob with bash scripting and still getting used to all the options available originally coming from a windows background couch, couch... Now I am more than happy in NIX world SmilieSmilie

Anyway, the script is working exactly the way I want it and is formatting nicely, however, I would like to know if there are any refinement that can be suggested to clean the script up and make it more efficient and cleaner.

Code:
#!/bin/bash
clear

srvice[0]="plexmediaserver"
srvice[1]="plexconnect"
srvice[2]="plexpy"
srvice[3]="nzbdrone"
srvice[4]="couchpotato"
srvice[5]="headphones"
srvice[6]="utserver"
srvice[7]="qbittorrent-nox"
srvice[8]="HTPC-Manager"
srvice[9]="sabnzbdplus"
srvice[10]="webmin"


for i in "${srvice[@]}";  do
printf "$i: " | sed 's/.*/\u&/' | sed -e :a -e 's/^.\{1,16\}$/& /;ta' && systemctl is-active $i | sed 's/failed/inactive/g' | sed 's/.*/\u&/';
done

Output looks like this (with nice padding between)
Code:
Plexmediaserver: Active
Plexconnect:                  Active
Plexpy:                        Activating
Nzbdrone:                     Active
Couchpotato:                 Active
Headphones:                 Active
Utserver:                      Active
Qbittorrent-nox:            Inactive
HTPC-Manager:             Inactive
Sabnzbdplus:                Active
Webmin:                      Active


I welcome your suggestions and advice - please be general with me Smilie

Cheers,
Darren

Last edited by Simplify; 01-01-2017 at 12:28 PM..
# 2  
Old 01-01-2017
Ok. For a good chance to get help you can describe what your script should do. And show the normal output of the script. This makes it easier for the helping guys to understand what you are doing.

After reviewing your script, you are generating a services list with uppercase first letters of service names and service states.

Recommendations:
  • You may assign your array in a single line like this myservices=("plexpy" "couchpotato" ... )
  • You do not need sed to transform to uppercase. Bash can do that itself without spawning a new process ${VAR^}. Spawning Subprocesses cost far more system resouces than using interpreter(=bash here) functions.
  • Simple substitutions can be done by bash too: ${VAR/SEARCHPATTERN/REPLACE}
Your Code may look like this:


Code:
srvice=("sabnzbdplus" "webmin")

for i in "${srvice[@]}";  do
   state="$(systemctl is-active)"
   state="${state/failed/inactive}"
   printf "${i^}: ${state^}\n"
done

The above script creates 12 Processes, while the original creates 56.

Last edited by stomp; 01-01-2017 at 12:46 PM..
These 2 Users Gave Thanks to stomp For This Post:
# 3  
Old 01-01-2017
Thanks Stomp!

Apologies for not explaining the process in the first place... you are 100% correct on the purpose of the script that I will then using in conjunction with Conky.

In fact, since my original post, I have been working further on the logic that I have been using and had started down the track you have shown, however not as concisely as you have posted. Here is what I have finished with at the moment... Is there another (better) way of getting the nice padding into the layout (I've currently used sed to achieve this as you can see) And you can also see i have refined the logic a little due to not getting an exact response from the systemctl command - sometimes you will get "fail", active, or sometimes it will be "activating" so I have simplified the output as you can see.

Code:
#!/bin/bash
clear

srvice=("plexmediaserver" "plexconnect" "plexpy" "nzbdrone" "couchpotato" "headphones" "utserver" "qbittorrent-nox" "HTPC-Manager" "sabnzbdplus" "webmin");

for i in "${srvice[@]}";  do
   systemctl is-active $i > /dev/null 2>&1
   if [ $? -eq 0 ]; then state="Live"; else state="Dead"; fi
   printf "${i^}: "| sed -e :a -e 's/^.\{1,16\}$/& /;ta' && printf "${state^}\n"
done

Further comments and suggestions very welcome Smilie

Cheers,
Darren

Last edited by Simplify; 01-01-2017 at 03:03 PM..
# 4  
Old 01-01-2017
That for the state assignment...
Code:
state=$(systemctl is-active $i >/dev/null && echo Alive || echo Dead)

What's regarding the output, printf kann handle format strings as described in the manpage: man 3 printf.

Example

Code:
printf "%-20s : %-20s\n" "$service_name" "$service_state"

Description

Output String variable $service_name with minimum length 20 chars - aligned left - then space then : then space then variable $service_state with minimum length 20 chars - aligned left and then new line.
This User Gave Thanks to stomp For This Post:
# 5  
Old 01-01-2017
I want to add some different angle:

It is a laudable effort to try to streamline scripts, but - like in any software engineering task - you also need to concentrate on the maintainability of the code you write. Your script is not very long, so some of my points will make not that big of an impact, but imagine scripts of several hundreds or even thousands of lines (i have some of these). You can't start early enough to develop healthy habits. Here are a few tips.

Avoid long lines!
I know, it is really cool to write lines like

Code:
printf "$i: " | sed 's/.*/\u&/' | sed -e :a -e 's/^.\{1,16\}$/& /;ta' && systemctl is-active $i | sed 's/failed/inactive/g' | sed 's/.*/\u&/';

but, honestly: suppose you haven't looked at that for a year and want to change something. Do you think you could immediately say what the line is doing? I don't think so. How about this:

Code:
if printf "$i: " | sed 's/.*/\u&/' | sed -e :a -e 's/^.\{1,16\}$/& /;ta' ; then
     systemctl is-active $i | sed 's/failed/inactive/g' | sed 's/.*/\u&/'
fi

Better, no? As a general rule of thumb: i avoid lines longer than 80 characters. Exceptions are the definition of constants, but everything else can in 99 out of hundred times be broken down and made more readable.

Avoid sed|sed, sed|awk, grep|sed and the like
UNIX is rich in various textfilters: grep, awk, sed, tr, .... If you use one for a certain purpose: stick to it. Whenever you do "sed | sed" it can be put into one sed-command. This is your code:

Code:
printf "$i: " | sed 's/.*/\u&/' | sed -e :a -e 's/^.\{1,16\}$/& /;ta'

How about this ( have streamlined the sed-code too):

Code:
printf "$i: " | sed 's/^/\u/
                     s/$/               /
                     s/^\(.\{16\}\).*/\1/'

(Not taking into account that you could do all that with printf too:)

Code:
printf "\u%16s: " "$i"

Comment your code
Chances are that, even though you know your script right now, you will have forgotten its inner workings in some time. This is why you need to comment your code so that in a year, when you want to change something, the time needed to understand your own code is minimised. Again: in a 20-lines script this could be neglected, but once your scripts get longer it is vital. In a 1000-lines script it can make the difference between a change taking 5 minutes, five hours or even five days. How a script of mine looks like can be seen here. I still use the same header whenever i start a new script.

I hope this helps.

bakunin
This User Gave Thanks to bakunin For This Post:
# 6  
Old 01-02-2017
Thanks again Stomp, that has certainly explained things. I have been looking at the operations you provide, however, I was still having some problems with the syntax. With your example, I now understand that process and will be using that now and in the future.

Quote:
Originally Posted by stomp
That for the state assignment...
Code:
state=$(systemctl is-active $i >/dev/null && echo Alive || echo Dead)

What's regarding the output, printf can handle format strings as described in the manpage: man 3 printf.

Example

Code:
printf "%-20s : %-20s\n" "$service_name" "$service_state"

Description

Output String variable $service_name with minimum length 20 chars - aligned left - then space then : then space then variable $service_state with minimum length 20 chars - aligned left and then new line.
# 7  
Old 01-02-2017
Off-Topic

Quote:
Originally Posted by bakunin
In a 1000-lines script it can make the difference between a change taking 5 minutes, five hours or even five days. How a script of mine looks like can be seen here. I still use the same header whenever i start a new script.
I agree to learning a good programming style is helpful in many ways. I may add that choice of the programming language is one of the questions of good programming style too. I myself decided that at projects of a certain size, bash or shell scripting in general is a pain in the ass, due to the lack of efficient programming possibilites(clean function calls only as subprocess and ineffiency in general), an annoying quoting mess and a the default that all variables are global(yes, there's local too, I know). You need a very good discipline to write larger shell scripts, or you're sooner than later in a bloody mess.

I wrote some some larger shell scripts - with the experience of certainly some 100K lines Shell-Scripting or more - because it was easier to start with, even thinking it through with my goal in mind and believing that it might work with Bash. But at a certain point it's ineffiency compared to any scripting languages made me regret the decicision to use a Shell Script.

And yes - every case needs it's review. That's not a general expression of "Shell is improper for any larger program".

The charm is of course it's platform indepency. Nothing to install. A Shell/Bash is everywhere available. Of course a truly platform independent script must include extensive Checking of the environment for all program calls and it's options besides those provided by the basic set and strictly sticking to standard conform methods and feature sets. But still no real satisfaction to write such a monster like INXI, which I consider as good tool for the job it is designed to do(Even the author of INXI wrote in its README on github, that he does not like it, but that it is the best choice for his goals).

Last edited by stomp; 01-02-2017 at 01:17 AM..
Login or Register to Ask a Question

Previous Thread | Next Thread

7 More Discussions You Might Find Interesting

1. Shell Programming and Scripting

How to block first bash script until second bash script script launches web server/site?

I'm new to utilities like socat and netcat and I'm not clear if they will do what I need. I have a "compileDeployStartWebServer.sh" script and a "StartBrowser.sh" script that are started by emacs/elisp at the same time in two different processes. I'm using Cygwin bash on Windows 10. My... (3 Replies)
Discussion started by: siegfried
3 Replies

2. Shell Programming and Scripting

Shell script works fine as a standalone script but not as part of a bigger script

Hello all, I am facing a weird issue while executing a code below - #!/bin/bash cd /wload/baot/home/baotasa0/sandboxes_finance/ext_ukba_bde/pset sh UKBA_publish.sh UKBA 28082015 3 if then echo "Param file conversion for all the areas are completed, please check in your home directory"... (2 Replies)
Discussion started by: ektubbe
2 Replies

3. UNIX for Dummies Questions & Answers

Calling a script from master script to get value from called script

I am trying to call a script(callingscript.sh) from a master script(masterscript.sh) to get string type value from calling script to master script. I have used scripts mentioned below. #masterscript.sh ./callingscript.sh echo $fileExist #callingscript.sh echo "The script is called"... (2 Replies)
Discussion started by: Raj Roy
2 Replies

4. Shell Programming and Scripting

Script will keep checking running status of another script and also restart called script at night

I am using blow script :-- #!/bin/bash FIND=$(ps -elf | grep "snmp_trap.sh" | grep -v grep) #check snmp_trap.sh is running or not if then # echo "process found" exit 0; else echo "process not found" exec /home/Ketan_r /snmp_trap.sh 2>&1 & disown -h ... (1 Reply)
Discussion started by: ketanraut
1 Replies

5. Shell Programming and Scripting

SOLVED: Refining an awk command

I have a file (file1) with in the below format ST*820*212121 BPR*C*213212.20*C*212*CCD*01***01*071000013*DA*321321*101208 TRN*1*21321321*13213 N1*PR*3232. dff. SYS.*91*3232 ENT*1 N1*PE* 2132121321 RMR*TN*234456677888**192387.20*192387.20 REF*IV*234456677888*213213 3213 UNI... (0 Replies)
Discussion started by: Muthuraj K
0 Replies

6. Shell Programming and Scripting

Refining if loops using sed/awk

hi All, I have the following two requirements: case 1: In a file i have the below code: if ((a>b)) a=b; else a = c; by using some means i need to convert the line to the following output: Output required: case 2: In a file i have the below code: if (a>b) a=b; else a... (4 Replies)
Discussion started by: engineer
4 Replies

7. Shell Programming and Scripting

create a shell script that calls another script and and an awk script

Hi guys I have a shell script that executes sql statemets and sends the output to a file.the script takes in parameters executes sql and sends the result to an output file. #!/bin/sh echo " $2 $3 $4 $5 $6 $7 isql -w400 -U$2 -S$5 -P$3 << xxx use $4 go print"**Changes to the table... (0 Replies)
Discussion started by: magikminox
0 Replies
Login or Register to Ask a Question