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 :b::slight_smile:

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.

#!/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)

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 :o

Cheers,
Darren

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:

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.

2 Likes

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.

#!/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 :slight_smile:

Cheers,
Darren

That for the state assignment...

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

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.

1 Like

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

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:

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:

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

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

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

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

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

1 Like

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.

Off-Topic

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).

I agree with both of you regarding your comments and i will post a more detailed response shortly.

---------- Post updated at 01:35 PM ---------- Previous update was at 01:34 PM ----------

Now i have to post another response to allow me to send URL's in my detailed response :slight_smile:

---------- Post updated at 01:38 PM ---------- Previous update was at 01:35 PM ----------

ok more detailed coming next this forum software must have a bug as i have edited the offending URL add in my detailed post and it still will not let me post. Lets try again.

---------- Post updated at 01:40 PM ---------- Previous update was at 01:38 PM ----------

Ok here is what i have gone with now;

#!/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
   state=$(systemctl is-active $i >/dev/null && echo Alive || echo Dead)
   #state="${state/failed/inactive}"
   printf "%-18s : %-20s\n" "${i^}" "${state^}"
done

---------- Post updated at 09:15 PM ---------- Previous update was at 01:40 PM ----------

Interestingly I have found that much of the script isn't needed with conky, however, it has been a valuable learning experience.

---------- Post updated at 09:16 PM ---------- Previous update was at 09:15 PM ----------

Hi bakunin, thank you for your comments and suggestions. I agree with your comments. I tend to notarize my scripts so that years later I do understand what I was doing at the time when I put the script together and also as technology evolves I can update it and make it better or more streamlined or reduce the number of processes as Stomp demonstrated.

Being reasonably new to the NIX platform and coding within it and I don't often need to do a lot of coding, I thought I would attempt to put good scripts together from the outset. I also have to mentioned of all the scripting languages I have used over the years BASH is fantastic and fast becoming my favorite - I do also like JavaScript.

Thanks for the comments and examples using sed - i didn't know you could do that - very valuable and i will be using that in future also.

Regarding printf - whilst putting this script together, i have been doing a lot of reading and looking at different examples around the internet and have found this command to be very powerful and prefer to keep scripts as super simple as possible and certainly where possible would prefer to use printf for formatting rather than adding another process of command into the logic. I was having trouble getting the syntax correct using printf (obviously my lack of knowledge and understanding) hence the reason I first attempted with sed.

Sorry, that is not a bug but a feature: as a means to fight spammers who occasionally swamped the forum with links for "goods of questionable reputation" (fake passports, viagra pills, ... take your pick) we took away the right to post links for users below a certain post count. Once you are over that threshold (and it is going to be soon) you can post links without problem. This way we drove the worst spammers away and with the rest we can deal. Sorry for this inconvenience, but there we foud no better alternative.

Thank you, i am glad i could be of help.

Thinking again, here are some other tips and a response to stomp:

First: yes, one needs a lot of discipline. It turns out that the "freedom" the shell grants you (like not having to declare your variables, etc.) is best not used at all. One should write shell as if it was just another high-level language (like C or FORTRAN - are there any others?). This is why i prefer Korn Shell over bash, why i always declare my variables before i use them and why i write like shell variables were as typed as C variables.

In C it is possible to create a string, use it as an integer and - if the value just happens to match - as a pointer thereafter. This "clever programming" usually gets you into deep kimchi sooner than later. The same is true for shell scripts. I think the "obfuscated C contest" is a funny pastime to show off your skills - but whatever you write professionally should be the direct opposite of that: clear, self-evident, easy to read.

Itmaywellbethatyoucanstilldecipherwhatiwritethisway but perhaps employing this style consistently would not help to make me your favourite author.

About the topic of scripting versus programming (high.level languages): both have their place. As a systems administrator you need all kinds of administrative scripts/procedures and because i like my procedures to be as reliant as possible probably 80% of my code deals with error handling and logging. If you have an environment with ~400 systems (my last project) and write a script to create user accounts you check:

  • if the user already exists (and, in case it doesn't)
  • if the user ID is already taken on that server
  • if the user name is already taken
  • if the parent directory for the intended HOME already exists
  • if there is enough room to create the HOME
  • add other possible problems here ad libitum....

And my script checks all these on every server before even attempting to create the account. A script doesn't need to be able to deal with all these problems, but if on server 347 the creation has failed i rather read a log entry of user name "foobar" already taken, cannot continue than " mkuser error: stop "

All the tasks above are easy to do in script and, after doing all these checks, writing a documentation in the header, etc., the final script (yes, i use this every day, upon request i'll post it) is about 550 lines long (i make heavy use of a self-written library of external shell functions, in bash these functions would add another ~1000 lines of code). But the same in some highlevel language would be more closely to 10k lines and would take several months to write. This was written in about 3 days.

It is, IMHO, not about the size of a programming project, but its scope, that will determine the best-suited language. Like nobody would write a bookeeping software in assembler (not any more) and nobody would write number-crunching programs in anything else than FORTRAN, nobody would write application programs in script or administrative scripts in any high-level language. Its just not the right tool for the task.

To sum up about script programming: take it serious. Take it as serious as you would write any other piece of software because that in fact it is. With a different scope and different means, but the principles of good software engineering apply here no less than in any other environment. Even more so because the language doesn't enforce any strict work so that you have to make up with discipline what the language lacks in strictness.

I already stressed to comment your code: declaring your variables gives you the opportunity to describe their contents, which i always find helpful. Consider this code:

#! /bin/yourshell

typeset    chLine=""                      # line to process
typeset -i iArrCnt=1                      # counter to current array element
typeset    fIn="/bla/foo/somewhere"       # default input file
typeset    fOut="/bla/foo/whatever"       # default output file

# main ()
... rest of code

You might not like the hungarian style notation i adopted and stuck with but suppose you get this script in hand and should change something: it might be helpful to understand immediately what in which variable is supposed to be. It is not relevant which standard you use - but use any one (even your own) and stick with it consistently.

My own style (but that is personal preference more than anything else and heavily influenced by old habits from years of programming in assembler and FORTRAN) is to declare all local variables at the start of each function, put comments at position 50 of the line and never let a line be longer than 80 characters if i can avoid it. I use 5 spaces indentation and write my loops and control-structures in one line:

while [ "$bla" = "$foo" ] ; do
     this "$bla"
     that "$foo"
done

case $bla in
     X)
          this X
          that Y
          ;;

     *)
          bla
          if [ -z "$whatever" ] ; then
               whatever
          else
               something ELSE
          fi
          foo
          bar
          ;;

esac

But it doesn't matter so much what you do as long as you do it consistently. Programming is about organising your thoughts in the strictest possible way and your code should show this organisation.

Finally: Unix programs should maintain some tenets of interoperability. For instance, your script:

#!/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
   state=$(systemctl is-active $i >/dev/null && echo Alive || echo Dead)
   #state="${state/failed/inactive}"
   printf "%-18s : %-20s\n" "${i^}" "${state^}"
done

Suppose you would use this script in some sort of pipeline or with a redirection to a log file:

script.sh | ...
script.sh >/path/to/log 2>&1

Don't you think that clear -statement, as useful as it might be in interactive use, might maybe disturb the processing? I would leave that out for exactly this reason.

Another topic: strive to adhere to the POSIX standard as much as you can. This is sometimes cumbersome and bound to additional effort, especially when you use Linux, because there are lots of nice little extras in many commands that come in handy. Still, it might limit the usability of your script to a certain platform i you use the extensions this platform offers for the standard. But using the bare standard every time is oftenly rewarded when you take a script from platform X, copy it to a completely different platform and it runs without any change.

OK, enough for today and happy scripting.

I hope this helps.

bakunin

1 Like

Hi bakunin,

Thanks for your time and comments - they are welcomed :slight_smile:

I did have a go at some more structured programming in formal programming in Pascal, C and basic (as well as some others) languages some 25 years ago, however, I didn't do anything series with the skills. These days whilst I enjoy the scripting aspect of having a problem and creating a solution to deal with it, I would tend to stick to scripting, however, I am always looking at streamlining and clean simple structure with my scripts. You have certainly given me some things to consider and remember

I was not aware of KSH - so on your mention i went and had a quick read - I like it! So i have installed it and will start doing some scripting using korn and see how I go :slight_smile:

I may have to ask you for some more tips from time to time in the future :wink:

All the best for the new year!

Cheers,
Darren

Yes! One more converted! :wink:

Seriously: i am glad you like it. What i appreciate most about Korn Shell is the possibilty to set the variable FPATH, which works similar to PATH - though not for binaries but shell functions. This way you can designate a directory with functions written in shell which you can use from any of your scripts. This way i built a "library" over the years with functions for repetitive tasks: writing a log, create/remove space for temporary files, check if the script is run with root authority ....

You are welcome. Ask as much as you wish. Here is a tip you haven't asked: Barry Rosenberg, "KornShell Programming Tutorial" and "Hand-On KornShell93 Programmming". These are not only very good books about the topic but also great fun to read. You might want to give them a try.

I hope this helps.

bakunin

1 Like

Sounds good i am going to get into some more scripting over the next few weeks :wink:

The important thing i like about KSH is POSIX compliant! :slight_smile: :b:

Funny you mention books I was looking at a few online today :wink: I will add those to my book list to pick up in the near future.

Tell me bakunin, have you or do you use Conky or perhaps something else that you may suggest I look at. I have settled on Lubuntu as the Linux platform I prefer (at least at present). I have a number of Virtual machines that I run and Lubuntu if very good with low resource allocations. In the past, I have played with Solaris (when it was SUN and I liked it a great deal as a Unix Platform) and I did have a have a passing look at SUSE before Novel got hold of it. :o

Anyway, I digress - how this original script thread started was I was looking to use Conky to monitor some of the services I use on my HTPC Lubuntu system to so at a glance I can see if there is anything I need to give attention to...

And Stomp and other visitors are now reading and starting to roll their eyes as we have really gone off topic :eek:

Hi @bakunin

Ok, I have a new scripting requirement that I wanted to run by you...

I have a 3rd party script that pulls metadata from imdb.com and gives it to me in a clean text format. Now I need to modify the format and some of the fields to make the data ready for the next process I want to use it for - that is applying it to a mp4 file to so that my iTunes reads it correctly.

No, the question is - script in bash, ksh, sh or python? I think I would need to use grep or sed to manipulate the string i get from the 3rd party script output and if your suggestion is bash/ksh/sh then what do you suggest - grep or sed r a combination of both? OR do you think it could all be down via shell?

look forward to your thoughts

PS for anyone else reading, your thoughts and comments are welcome and sorry for going off topic - I would have sent bakunin a PM, however, i haven't posted 10 times yet...

Cheers,
Darren

---------- Post updated at 08:52 PM ---------- Previous update was at 08:30 PM ----------

Oh and I just thought about stomps comments - please see what I am attempting to accomplish below.

this is an output example I get from the 3rd party script (GitHub - bgr/imdb-cli: Command line tool for retrieving IMDb movie information)

Title:Deep Throat
Year:1993
Rated:TV-14
Released:17 Sep 1993
Season:1
Episode:2
Runtime:46 min
Genre:Drama
 Mystery
 Sci-Fi
Director:Daniel Sackheim
Writer:Chris Carter (created by)
 Chris Carter
Actors:David Duchovny
 Gillian Anderson
 Jerry Hardin
 Michael Bryan French
Plot:Mulder and Scully investigate the mysterious case of a military test pilot who disappeared after experiencing strange psychotic behaviour.
Language:English
Country:USA
Awards:N/A
Poster:http://ia.media-imdb.com/images/M/MV5BODM2NzAwMjgxNF5BMl5BanBnXkFtZTcwNzYwNjkzMQ@@._V1_SX300.jpg
Metascore:N/A
imdbRating:8.3
imdbVotes:3385
imdbID:tt0751099
seriesID:tt0106179
Type:episode
Response:True

and this is what I need to make it look like so that I can use FFmpeg to apply it to the mp4 file

;FFMETADATA1
major_brand=qt  
minor_version=512
compatible_brands=qt  
title=This Guilty Blood
album=Shadowhunters, Season 2
genre=Action
track=1/10
disc=2
date=2017-01-02
synopsis=Only hours have passed since Jace left with Valentine and all hell has broken loose at The Institute. Alec, Isabelle and Clary are desperate to find Jace, but are quickly stopped in their tracks with the arrival of Victor Aldertree, who means business about getting the Institute back on track. But getting the New York Shadowhunters in-line with The Clave may be counterintuitive to Alec, Isabelle and Clary's plan to rescue Jace. Meanwhile, Jocelyn has a lot to catch up on now that she is awake.
iTunEXTC=us-tv|TV-14|500
iTunMOVI=<?xml version\="1.0" encoding\="UTF-8"?><!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"><plist version\="1.0"><dict>\
<key>cast</key><array>\
<dict><key>name</key><string>Katherine McNamara</string></dict>\
<dict><key>name</key><string>Dominic Sherwood</string></dict>\
<dict><key>name</key><string>Matthew Daddario</string></dict>\
<dict><key>name</key><string>Alberto Rosende</string></dict>\
<dict><key>name</key><string>Isaiah Mustafa</string></dict>\
</array>\
<key>screenwriters</key><array>\
<dict><key>name</key><string>Michael Reisz</string></dict>\
</array>\
<key>directors</key><array>\
<dict><key>name</key><string>Matt Hastings</string></dict>\
</array>\
</dict></plist>\

media_type=10
show=Shadowhunters
episode_id=This Guilty Blood
season_number=2
episode_sort=1
network=Freeform
hd_video=0
description=Only hours have passed since Jace left with Valentine and all hell has broken loose at The Institute.
encoder=Lavf57.25.100

Please start a new thread to discuss a new topic (such as your new script) instead of just using one thread to discuss several topics. (Using a single thread makes it hard for readers to figure out to which topic future posts refer.)

And, please, when showing sample input and sample output, make the sample output be output that you want to produce when given the sample input you showed earlier. I don't see any clear way to transform input data for a show named Deep Throat into output for a show named ShadowHunters and assume that is not what you expect your script to do. With the examples you provided in post #13 we have no way to determine whether output fields like major_brand=qt and compatible_brands=qt are constants to be provided by the conversion process or are derived from data present in some input but not provided in others.

Please do not continue discussion on post #13 in this thread!

2 Likes