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

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

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.

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.

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.

Agreed -- thanks for the expansion upon that (bold text by me)

Cheers,
Sasha

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

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:

I'm pretty sure this will work, but I haven't tested it:
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

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.

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:

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:

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