LinuxQuestions.org
Latest LQ Deal: Complete CCNA, CCNP & Red Hat Certification Training Bundle
Go Back   LinuxQuestions.org > Forums > Linux Forums > Linux - Newbie
User Name
Password
Linux - Newbie This Linux forum is for members that are new to Linux.
Just starting out and have a question? If it is not in the man pages or the how-to's this is the place!

Notices


Reply
  Search this Thread
Old 09-15-2009, 04:21 PM   #1
Aseem
LQ Newbie
 
Registered: Nov 2005
Location: India
Posts: 6

Rep: Reputation: 0
Make a simple script more concise


Hi,

Is it possible to make the below mentioned script more concise.
Either commands can be merged or some other command that can be used to make it small, like a one line script.


#! /bin/bash

z=0
y=`find /home/logs/servers/ -iname engine.log* -prune -mmin -10 -type f -print`

x=`tail $y|awk '/msg/ {print $23}'`

z=`echo $x | awk '/ / {print $1}'`

if [ $z -le 120 ]; then
mailx -s "msg/sec less than 120" a@b.com < /dev/null
else
mailx -s "msg/sec greater than 120" a@b.com < /dev/null
fi



Thanks
 
Old 09-16-2009, 09:01 AM   #2
kbp
Senior Member
 
Registered: Aug 2009
Posts: 3,790

Rep: Reputation: 650Reputation: 650Reputation: 650Reputation: 650Reputation: 650Reputation: 650
Why do you need to compress it ? .. personally, I prefer to write my scripts in an expanded format to ensure they remain comprehensible. It makes it hard for anyone else to maintain and/or understand if you use obscure functionality to squash something into one line, try this for entertainment -

http://www.ioccc.org/main.html


my2c
 
Old 09-16-2009, 12:40 PM   #3
lutusp
Member
 
Registered: Sep 2009
Distribution: Fedora
Posts: 835

Rep: Reputation: 102Reputation: 102
Exclamation

Quote:
Originally Posted by Aseem View Post
Hi,

Is it possible to make the below mentioned script more concise.
Either commands can be merged or some other command that can be used to make it small, like a one line script.


#! /bin/bash

z=0
y=`find /home/logs/servers/ -iname engine.log* -prune -mmin -10 -type f -print`

x=`tail $y|awk '/msg/ {print $23}'`

z=`echo $x | awk '/ / {print $1}'`

if [ $z -le 120 ]; then
mailx -s "msg/sec less than 120" a@b.com < /dev/null
else
mailx -s "msg/sec greater than 120" a@b.com < /dev/null
fi



Thanks
YES -- you can squeeze your script into one line. but THEN you won't be able to understand or change it. It will become garbage.

A computer program has two primary purposes -- ONE is to be comprehensible to the computer. THE OTHER is to be comprehensible to the human.

As time passes, as computers become faster and faster, it becomes more important that a program remain understandable to the human who originally wrote it.

If I were you, I would be asking, "How can I make this script easier to understand and change later on?" That, by the way, is what separates professional software developers from others.
 
Old 09-16-2009, 12:48 PM   #4
GrapefruiTgirl
LQ Guru
 
Registered: Dec 2006
Location: underground
Distribution: Slackware64
Posts: 7,594

Rep: Reputation: 551Reputation: 551Reputation: 551Reputation: 551Reputation: 551Reputation: 551
Quote:
Originally Posted by lutusp View Post
...That, by the way, is what separates professional software developers from others.
I don't know.. You lump a pretty large chunk of people into that 'others' category. I would argue that there are lots of people who are great coders/scripters, who are not necessarily "professional software developers".

Writing readable, well commented scripts is not rocket science.

Last edited by GrapefruiTgirl; 09-16-2009 at 12:51 PM.
 
Old 09-16-2009, 01:12 PM   #5
lutusp
Member
 
Registered: Sep 2009
Distribution: Fedora
Posts: 835

Rep: Reputation: 102Reputation: 102
Quote:
Originally Posted by GrapefruiTgirl View Post
I don't know.. You lump a pretty large chunk of people into that 'others' category. I would argue that there are lots of people who are great coders/scripters, who are not necessarily "professional software developers".

Writing readable, well commented scripts is not rocket science.
True, but the OP here was asking how to move in the opposite direction. I wouldn't have posted otherwise.

As to "professional software developers," I should have thought that expression over. I know a lot of very talented amateurs, some who would resist becoming professionals because that would take all the fun out of it.
 
Old 09-16-2009, 01:18 PM   #6
catkin
LQ 5k Club
 
Registered: Dec 2008
Location: Tamil Nadu, India
Distribution: Debian
Posts: 8,576
Blog Entries: 31

Rep: Reputation: 1195Reputation: 1195Reputation: 1195Reputation: 1195Reputation: 1195Reputation: 1195Reputation: 1195Reputation: 1195Reputation: 1195
Quote:
Originally Posted by GrapefruiTgirl View Post
I would argue that there are lots of people who are great coders/scripters, who are not necessarily "professional software developers".
And there are lots of "professional software developers" who are not great coders/scripters!

But ... the more code you have had to maintain (including your own!), the more you understand the importance of clarity. That's one of the things that makes K+R's "The C Programming Language" such a masterpiece; they did not have the advantage of much established "good practice" but the examples are breathtaking in both their clarity and efficiency.
 
Old 09-16-2009, 01:19 PM   #7
GrapefruiTgirl
LQ Guru
 
Registered: Dec 2006
Location: underground
Distribution: Slackware64
Posts: 7,594

Rep: Reputation: 551Reputation: 551Reputation: 551Reputation: 551Reputation: 551Reputation: 551
Quote:
Originally Posted by lutusp View Post
True, but the OP here was asking how to move in the opposite direction. I wouldn't have posted otherwise.

As to "professional software developers," I should have thought that expression over. I know a lot of very talented amateurs, some who would resist becoming professionals because that would take all the fun out of it.
Agreed -- thanks for the expansion upon that (bold text by me)

Cheers,
Sasha
 
Old 09-16-2009, 01:21 PM   #8
GrapefruiTgirl
LQ Guru
 
Registered: Dec 2006
Location: underground
Distribution: Slackware64
Posts: 7,594

Rep: Reputation: 551Reputation: 551Reputation: 551Reputation: 551Reputation: 551Reputation: 551
Quote:
Originally Posted by catkin View Post
But ... the more code you have had to maintain (including your own!), the more you understand the importance of clarity.
True, I agree with what you said in its entirety

And, I'll be the first to admit that I like to make my code WORK first, and then I tend to compact and optimize it as though it really matters --often to my own chagrin later :|

Sasha
 
Old 09-16-2009, 01:23 PM   #9
forrestt
Senior Member
 
Registered: Mar 2004
Location: Cary, NC, USA
Distribution: Fedora, Kubuntu, RedHat, CentOS, SuSe
Posts: 1,288

Rep: Reputation: 99
OK, you probably want to make variables out of IMPORTANT information, not the ones you've used, which aren't even necessary. I also don't think this is any less clear than your code. It is (IMHO) more legible and more easily modified.

Also, you no longer need "-print" with find. From the man page:

Quote:
If the expression contains no actions other than -prune, -print is performed on all files for which the expression is true.
I'm pretty sure this will work, but I haven't tested it:
Code:
#! /bin/bash
SERVERDIR='/home/logs/servers/'
FILEMATCH='engine.log*'
MAILTHRESHOLD=120
MAILTO='a@b.com'

# look for files in SERVERDIR that ... (give good comment on what the code does so you know in 18 mos)
if [ `find ${SERVERDIR} -iname ${FILEMATCH} -prune -mmin -10 -type f | tail | awk '/msg/ {print $23}' | awk '/ / {print $1}'` -le ${MAILTHRESHOLD} ] ; then
        OPERATOR='less'
else
        OPERATOR='greater'
fi

mailx -s "msg/sec ${OPERATOR} than ${MAILTHRESHOLD}" ${MAILTO} < /dev/null
This is my style, and I have developed it over many years. It puts the "one-liner" where it needs to go, uses variables that are easily understood, and doesn't repeat code unnecessarily. I hope this demonstrates an example of "good" programming for you. I think it also answers lutusp's question, "How can I make this script easier to understand and change later on?" fairly well.

HTH

Forrest

Last edited by forrestt; 09-16-2009 at 01:52 PM. Reason: bug in code
 
Old 09-16-2009, 01:27 PM   #10
i92guboj
Gentoo support team
 
Registered: May 2008
Location: Lucena, Córdoba (Spain)
Distribution: Gentoo
Posts: 4,063

Rep: Reputation: 381Reputation: 381Reputation: 381Reputation: 381
Quote:
Originally Posted by GrapefruiTgirl View Post
I don't know.. You lump a pretty large chunk of people into that 'others' category. I would argue that there are lots of people who are great coders/scripters, who are not necessarily "professional software developers".

Writing readable, well commented scripts is not rocket science.
I will have to agree. I've met professional coders that write very ugly code, and first grade students whose C programs could be read with only some basic understanding of the general basics of procedural programming. It's like that topic about doctors, people say they all have a very bad handwriting

In which regards the script,
  • ditch out z=0, you are already assigning a valid value to it four lines below.
  • I'd move the command out of the if block, use the if block to set a VAR to either "less" or "greater", then outside the if use "$VAR" in that command.
  • the final purpose of the three variables seems to get a valid value for $z, you might want to reutilize a single variable or combine these three statements into a single one. You can still make it readable by breaking it into many lines by escaping the line feed with \
  • you should really consider quoting variables that are going to hold file names, it will save you from more than one problem

All in all, I think that all that find/awk magic should be simplified, besides many other things, however I'd need to see exactly a relevant part of the log file(s) that it's parsing to be sure of what are you really wanting to achieve.

And, as others say, I'd care more about readability than about fancy obscure stuff. In two weeks you might be wondering what the heck this is when you look at the script again.
 
Old 09-16-2009, 05:31 PM   #11
Aseem
LQ Newbie
 
Registered: Nov 2005
Location: India
Posts: 6

Original Poster
Rep: Reputation: 0
Thanks for all the suggestions.

I do care for good programming practices and good documentation.
I am just testing/learning something and hence wanted to know whether it is possible to make it much shorter and yes a little obfuscated.

My log file contains entries like this:

20090916163703 : xyz.com : NNN : A111 : 5 : Data : nn : Cannot read event file FileManager >> VALUE OF ISDATACOMPRESSED = false.
20090916163812 : xyz.com : NNN : ENF100 : 5 : Listener : nn : Total Messages 20,000 in 607803 millis. Engine Performance 32 msg/sec

I need to pickup out the msg/sec rate and keep a check that it does not go beyond a specific value.

I tried to redo the code, here is it:

Code:
if [ "tail `find /home/nf/logs/servers/ -iname nfengine_FW.log* -prune -mmin -10 -type f`| awk '/msg/ {print $23}' | head -1" < 100 ] ; then
echo "WOW"
else
mailx -s "msg/sec greater than 120" a@b.com < /dev/null
fi
When i execute
tail `find /home/nf/logs/servers/ -iname nfengine_FW.log* -prune -mmin -10 -type f`| awk '/msg/ {print $23}' | head -1
I get proper output but i think its returning the output in string rather than numeric, because of which my if statement is not running.
 
Old 09-16-2009, 05:49 PM   #12
i92guboj
Gentoo support team
 
Registered: May 2008
Location: Lucena, Córdoba (Spain)
Distribution: Gentoo
Posts: 4,063

Rep: Reputation: 381Reputation: 381Reputation: 381Reputation: 381
Quote:
Originally Posted by Aseem View Post
Thanks for all the suggestions.

I do care for good programming practices and good documentation.
I am just testing/learning something and hence wanted to know whether it is possible to make it much shorter and yes a little obfuscated.

My log file contains entries like this:

20090916163703 : xyz.com : NNN : A111 : 5 : Data : nn : Cannot read event file FileManager >> VALUE OF ISDATACOMPRESSED = false.
20090916163812 : xyz.com : NNN : ENF100 : 5 : Listener : nn : Total Messages 20,000 in 607803 millis. Engine Performance 32 msg/sec

I need to pickup out the msg/sec rate and keep a check that it does not go beyond a specific value.

I tried to redo the code, here is it:

Code:
if [ "tail `find /home/nf/logs/servers/ -iname nfengine_FW.log* -prune -mmin -10 -type f`| awk '/msg/ {print $23}' | head -1" < 100 ] ; then
echo "WOW"
else
mailx -s "msg/sec greater than 120" a@b.com < /dev/null
fi
When i execute
tail `find /home/nf/logs/servers/ -iname nfengine_FW.log* -prune -mmin -10 -type f`| awk '/msg/ {print $23}' | head -1
I get proper output but i think its returning the output in string rather than numeric, because of which my if statement is not running.
That if has a couple of problems. '<' in bash is the redirection operator, you truly mean "-lt" (less than). And you also want this:

Code:
if [ $(tail $(find . -iname log) | awk '/msg/ {print $23}' | head -1) -lt 100 ] ; then
Also, I don't know the nature of the log file(s), but what the "head" is about? I guess you really want to check ALL the results, don't you? Not just a random one.

And finally, how do you determine that the results you are parsing at the current moment haven't been parsed before? How do determine that you did parse all of them? Maybe between one run and another some results will escape your eyes...
 
  


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
Need help getting started simple simple shell script dhonnoll78 Programming 6 12-17-2007 06:34 PM
How to make Simple Script to remove all files ending in mxm Marshalle Linux - General 2 12-25-2006 09:38 PM
How do I make a simple script and make it run on bootup?? coolblue Programming 5 12-18-2006 10:55 PM
I need to make simple script. sly_guy69 Linux - Newbie 4 01-22-2005 09:23 PM
How to make a simple addition in a shell script. philipina Programming 2 03-16-2004 05:38 AM


All times are GMT -5. The time now is 01:22 PM.

Main Menu
Advertisement
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
Facebook: linuxquestions Google+: linuxquestions
Open Source Consulting | Domain Registration