«

I entered the professional perl programming world by being paid
(that's what I call professional, though the code may be far from)
for
a very small perl script. The user basically wanted a config file
which contained as the first line a username, the second line a
password, and the remaining lines to be hotnames.
ex.
user
pass
127.0.0.1
127.0.0.2
Then I wrote the following script. It gathers the user, pass, and
hostlist, and then establishes an ssh connection to query a 'device'
and return the output in a file named after the host. Following is
that program:
#!/usr/bin/perl
# Code by
# For
# dmon-1.6
use warnings;
use strict;
use Net::SSH:erl;
my $cfgfile="./config";
open CONFIG, "<", $cfgfile || die $!;
chomp(my @cfgdat=(<CONFIG>));
my $user=shift(@cfgdat);
my $pass=shift(@cfgdat);
my $extcmd="ls -l";
my $stime=3;
while (defined $stime) {
foreach (@cfgdat) {
my $ssh=Net::SSH:erl->new($_);
$ssh->login($user,$pass);
my ($stdout,$stderr,$exit)=$ssh->cmd($extcmd);
open OUTFILE, ">>", $_ || die $!;
if ($stdout) {
print OUTFILE $stdout;
}
if ($stderr) {
print OUTFILE $stderr;
}
close OUTFILE;
}
sleep $stime;
}

I am just looking for critique. I have been a Unix Admin for over 15
years, and have used perl for one off scripts, but I spent time and
master Oreillys Learning Perl and Intermediate Perl (Mastering and
Advanced Perl are next) and am now looking to solely become a
commercial perl programmer. However, as I lack commercial experience,
I probably lack a 'standard' way of approaching things, or at least
don't know what experienced perl programmers know, which I'll learn
as
a function of time. Either way, if you have time, let me know how I
could have done all this better, and maybe even a source of
commercial
perl programs I can look at and see how pro's do it.
Ron

On Mar 10, 2:10*pm, Ben Morrow <b...@morrow.me.uk> wrote:
> Quoth "tbb!/fbr!" <ronaldljohn...@gmail.com>:
>
>
>
> > #!/usr/bin/perl
> > # Code by
> > # For
> > # dmon-1.6
> > use warnings;
> > use strict;
>
> Good.
>
> > use Net::SSH:erl;
> > my $cfgfile="./config";
> > open CONFIG, "<", $cfgfile || die $!;
>
> * * * * * * * *^^ Good * * * * *^^ Good
>
> That '||' doesn't do what you think it does: || has higher precedence
> than the RHS of open, so you are testing $cfgfile rather than the return
> value of open. You want either
>
> * * open(CONFIG, "<", $cfgfile) || die $!;
>
> or
>
> * * open CONFIG, "<", $cfgfile or die $!;
>
> The latter would be standard practice.
>
> Best practice is to use variables to hold filehandles rather than global
> barewords, and it's always a good idea to make error messages
> descriptive, so I would write that
>
> * * open my $CONFIG, "<", $cfgfile
> * * * * or die "can't read '$cfgfile': $!";
>
> If you are using perl 5.10 you can also put
>
> * * use autodie;
>
> up at the top and it will handle all these error checks for you.
>
> > chomp(my @cfgdat=(<CONFIG>));
>
> If $CONFIG becomes a variable this needs to change to match
>
> * * chomp(my @cfgdat = <$CONFIG>);
>
> > my $user=shift(@cfgdat);
> > my $pass=shift(@cfgdat);
> > my $extcmd="ls -l";
> > my $stime=3;
> > while (defined $stime) {
>
> I don't see you change $stime anywhere, so this will loop forever.
>
> > * foreach (@cfgdat) {
> > * * my $ssh=Net::SSH:erl->new($_);
> > * * $ssh->login($user,$pass);
> > * * my ($stdout,$stderr,$exit)=$ssh->cmd($extcmd);
> > * * open OUTFILE, ">>", $_ || die $!;
>
> As above, with the 'or' and the 'my $OUTFILE'.
>
> > * * if ($stdout) {
> > * * * print OUTFILE $stdout;
> > * * }
>
> I would probably have written that
>
> * * $stdout and print $OUTFILE $stdout;
>
> or even combined the two and written
>
> * * $_ and print $OUTFILE $_
> * * * * for $stdout, $stderr;
>
> but you would be perfectly entitled to consider that a little obscure.
> There's always a trade-off between writing for those who know the
> language well (and find the shortcuts make it easier to see through to
> the logic) and writing for those who don't (and find the shortcuts
> confusing).
>
> > * * if ($stderr) {
> > * * * print OUTFILE $stderr;
> > * * }
> > * * close OUTFILE;
>
> If you are writing to a filehandle, particularly for something like this
> that's likely to run unattended, you should check the return value of
> 'close' to make sure everything was written OK. There isn't any need to
> check the individual 'print's: because output is buffered, the error for
> a particular 'print' may not show up until later, but perl guarantees
> that if there was an error at any point it will be returned from
> 'close'.
>
> * * close $OUTFILE or die "can't write '$_': $!";
>
> (or use autodie, as above).
>
> > * }
> > * sleep $stime;
> > }
>
> > I am just looking for critique.
>
> That's a great deal better than most first posts here .
>
> > Either way, if you have time, let me know how I
> > could have done all this better, and maybe even a source of
> > commercial
> > perl programs I can look at and see how pro's do it.
>
> I don't know about 'commercial', but there's a lot of Perl code
> distributed with the perl core, and a whole lot more on the CPAN. Some
> of it is good, some bad, most indifferent, but there's nearly always
> something to be learned from seeing how other people have solved similar
> problems in the past (even if sometimes what you learn is 'don't do it
> like that').
>
> IMHO the only things that really matter are first, making sure you've
> solved the whole problem, including any nasty corner cases, and second,
> that you read over the code after you've written it, just as you would a
> piece of English, and make sure it makes the logic as clear as it can.
>
> Ben

Yeah, the $stime thing was originally a while (1) loop. $stime was
made variable for the owner of the script so that he can change how
often the loop runs. The infinite loop was by design.

Points taken on || vs. or and how it's properly evaluated.

I was originally getting error messages on the print OUTFILES when
either $stdout or $stderr wasn't set (always an error, though i
originally ignored it, so to suppress the error of the one that wasn't
set, I quickly simply tested to see if the variable was defined and if
it was, then write out the contents. Of course both $stdout and
$stderr were being written to the same file but I'm still sort of
learning, and wasn't sure how or if I could or even should combine
those two if tests.

Will make sure I check the status of closing files in the future.

Yeah, I got paid $100 for that, and that's just the beginning. I
wanted to write the guy a routine to automatically parse $stdout,
trigger on regex match, and then alert, but they were not interested.
I guess I mine as well do it just for fun.

I really appreciate that. I'm going to make those changes.

And I really like this to get rid of those two if's:

$_ and print $OUTFILE $_
* * * * for $stdout, $stderr;

I had to look up the contsruct, as 'Learning Perl' and 'Intermediate
Perl' by Oreilly briefly touch on shortcuts like this. I completely
understand the logic after a little further reading. I don't want to
just 'use' other people's code, at least not without fully
understanding it and being able to duplicate it on my own.

Thanks again.

Ron

>>>>> "tbb" == tbb!/fbr! <> writes:

tbb> Yeah, I got paid $100 for that, and that's just the beginning. I
tbb> wanted to write the guy a routine to automatically parse $stdout,
tbb> trigger on regex match, and then alert, but they were not
tbb> interested.

Like "swatch"? (Google for "swatch perl" for links.)

--
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<> <URL:http://www.stonehenge.com/merlyn/>
Smalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.
See http://methodsandmessages.posterous.com/ for Smalltalk discussion

On Mar 10, 5:48*pm, mer...@stonehenge.com (Randal L. Schwartz) wrote:
> >>>>> "tbb" == tbb!/fbr! *<ronaldljohn...@gmail.com> writes:
>
> tbb> Yeah, I got paid $100 for that, and that's just the beginning. I
> tbb> wanted to write the guy a routine to automatically parse $stdout,
> tbb> trigger on regex match, and then alert, but they were not
> tbb> interested.
>
> Like "swatch"? (Google for "swatch perl" for links.)
>
> --
> Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
> <mer...@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
> Smalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.
> Seehttp://methodsandmessages.posterous.com/for Smalltalk discussion

You have no idea how many times I have read your books. I would have
loved to take the courses, but outside of my budget. But I did get a
lot of satisfaction teaching myself with the help of you, brian, tom,
et al through the Oreilly books. I have read lots of books on manu
subjects, but the Intermediate Perl books descriptions of references
(or pointers in other languages for that matter) is the absolutely
BEST description (through the teaching) of the subject matter ever
written. And I have read many others books. IMO.

That little script I will modify on my own for the alerting stuff. I'm
so sick of the overengineered monitoring tools out there (except for
the original Unix version of Big Brother) that I want to write
something tight and fast on my own for my own purposes.

I'm familiar with swatch as I've many years in Unix. Logfile parsing
is older than dirt. I've spent the last year devoted to learning perl,
and I am glad I invested that time. There's a big move in the industry
for custom monitoring and alerting mechanisms due to the bloated
offerings from Nagios, Netsaint, Openview, Sun Management Center etc
etc etc etc, which simply do not work out the box for large
enterprises due to the fact that when you point a 400 node hostlist at
one of these monitoring softwares, you'll spend the next two months
tweaking false positives, and another 2 months of fighting with
developers who want some custom conditional monitoring solution.

I've much to learn in perl. I read learning perl until i had the whole
book memorized (it's how i learn). I did the same with Intermediate
Perl. Next is 'Advanced' and 'Mastering'. It takes me about 3 months
to memorize a book cover to cover. Well sub 300 page books anyways.

I'm honored by your post, btw. I know, no big deal, but your name is
on books all over my house and your a celebrity to me.

Ron

On Mar 10, 8:06*pm, "tbb!/fbr!" <ronaldljohn...@gmail.com> wrote:
> On Mar 10, 5:48*pm, mer...@stonehenge.com (Randal L. Schwartz) wrote:

Here's some more code I wrote for someone wanting 'random' scheduled
Twitter posts. Don't ask why, I don't even know.

I'm just wondering how the pro's would do this. Or just critique, or
even avenues of further exploration and learning.

rjohnson@cpqv2000 ~
$ cat perl.plx
#!/usr/bin/perl

use strict;
use warnings;
use List::Util qw (shuffle);

my $str='1234567890!@#$
%^&*()qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJK LZXCVBNM_+{}|:"<>?~`-
=[]\;\',./';
my @bits=split(//,$str);
my @rnd=shuffle(@bits);
print @rnd;

#for (my $count = 140; $count >= 1; $count--) {
# my $index=rand @bits;
# my $element=$bits[$index];
# print "$element";
#}

You can see the commented out portion was probably the worst way to do
it. So after research I came up with the other way, which was more
elegant and made 100 times more sense (to me) and makes it much easier
to capture that @rnd output and do with it as I will without weird
conversions I would have gone through trying to mess with the
commented out portion (I would have used sprintf to get the string
into a variable, but it would have been nasty I think building a 140
char string 1 char at a time; hence the reason for the code above the
commented out portion; using perls builtin strengths and default
modules).

BTW, I do realize the functionality in the two routines are different.
The original version (commented out) allowed for repeated chars, the
other version does not.

Ron

Okay, I think I have a very ugly piece of code below. Please keep in
mind I'm a 'Learning Perl' to 'Intermediate Perl' programmer, and have
much to learn. I am doing a crapload of manipulations below, but I'm
sure it could all be done in a line or two, not the 6-8 lines I did it
in. I understand the way I did it, but again, am looking for options
on how someone more experienced would do it.

use List::Util qw(shuffle);
my $str='1234567890!@#$
%^&*()qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJK LZXCVBNM_+{}|:"<>?~`-
=[]\;\',./1234567890!@#$
%^&*()qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJK LZXCVBNM_+{}|:"<>?~`-
=[]\;\',./';
my @bits=split(//,$str);
my @rnd=shuffle(@bits);
my $stringer="@rnd";
$stringer=~s/\s//g;
$stringer=substr($stringer,0,140);

I'm doing all of that to get to that final line (with the substr). I
could explain it line by line, but I'm sure most of you can read
through it and figure the 'logic' out.

It's doing exactly what I want, and it works, and I wrote a whole
program with this piece programming to the Twitter API (Net::Twitter &
Net::Twitter::Lite) for automated scheduled tweets (cron'd).

Just looking for advice, critique, or even a suggestion of where I
should be concentrating my perl studies.

Thanks,
Ron

"tbb!/fbr!" <> writes:

[...]


> use List::Util qw(shuffle);
> my $str='1234567890!@#$
> %^&*()qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJK LZXCVBNM_+{}|:"<>?~`-
> =[]\;\',./1234567890!@#$
> %^&*()qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJK LZXCVBNM_+{}|:"<>?~`-
> =[]\;\',./';
> my @bits=split(//,$str);
> my @rnd=shuffle(@bits);
> my $stringer="@rnd";
> $stringer=~s/\s//g;
> $stringer=substr($stringer,0,140);

$stringer = substr(join('', shuffle(grep { $_ !~ /\s/; } split(//, $str))), 0, 140);

Especially, it makes more sense to throw the whitespace away before
shuffling etc than to pass it through to the second-to-last stage of
this 'processing pipeline' where it is deleted from the string.

In article
<f061a0f9-dd9a-4801-882d->,
tbb!/fbr! <> wrote:

> Okay, I think I have a very ugly piece of code below. Please keep in
> mind I'm a 'Learning Perl' to 'Intermediate Perl' programmer, and have
> much to learn. I am doing a crapload of manipulations below, but I'm
> sure it could all be done in a line or two, not the 6-8 lines I did it
> in. I understand the way I did it, but again, am looking for options
> on how someone more experienced would do it.

It helps to figure out your program if we know what you are trying to
do. In this case, it looks like you are trying to generate a string of
140 random characters.
>
> use List::Util qw(shuffle);
> my $str='1234567890!@#$
> %^&*()qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJK LZXCVBNM_+{}|:"<>?~`-
> =[]\;\',./1234567890!@#$
> %^&*()qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJK LZXCVBNM_+{}|:"<>?~`-
> =[]\;\',./';

Here is a way to build up your source string from pieces that are
easier to type and understand:

my $numbers = join('',(0..9));
my $letters = join('',('a'..'z'));
my $ucletters = uc $letters
my $punct = q(!@#$%^&*_+{}|:"<>?~`-=[];',./) . "()";
my $str = $numbers . $letters. $ucletters. $punct;
$str .= $str;

> my @bits=split(//,$str);
> my @rnd=shuffle(@bits);
> my $stringer="@rnd";
> $stringer=~s/\s//g;
> $stringer=substr($stringer,0,140);

What Rainer said. I would add that their aren't any whitespace
characters in your source, so their shouldn't be any in your output.

--
Jim Gibson

On 2012-04-03, Jim Gibson <> wrote:
> In article
> <f061a0f9-dd9a-4801-882d->,
> tbb!/fbr! <> wrote:
>
>> Okay, I think I have a very ugly piece of code below. Please keep in
>> mind I'm a 'Learning Perl' to 'Intermediate Perl' programmer, and have
>> much to learn. I am doing a crapload of manipulations below, but I'm
>> sure it could all be done in a line or two, not the 6-8 lines I did it
>> in. I understand the way I did it, but again, am looking for options
>> on how someone more experienced would do it.
>
> It helps to figure out your program if we know what you are trying to
> do. In this case, it looks like you are trying to generate a string of
> 140 random characters.
>>
>> use List::Util qw(shuffle);
>> my $str='1234567890!@#$
>> %^&*()qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJK LZXCVBNM_+{}|:"<>?~`-
>> =[]\;\',./1234567890!@#$
>> %^&*()qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJK LZXCVBNM_+{}|:"<>?~`-
>> =[]\;\',./';
>
> Here is a way to build up your source string from pieces that are
> easier to type and understand:
>
> my $numbers = join('',(0..9));
> my $letters = join('',('a'..'z'));
> my $ucletters = uc $letters
> my $punct = q(!@#$%^&*_+{}|:"<>?~`-=[];',./) . "()";
> my $str = $numbers . $letters. $ucletters. $punct;
> $str .= $str;

Seeing as the OP, after constructing a string, then converts it into an
array, why not skip straight to the array... at least with the easy
bits:

my @bits = (0..9, "a".."z", "A".."Z");
my $punct = q(!@#$%^&*_+{}|:"<>?~`-=[];',./) . "()";
push @bits, $_ for (split(//, $punct));


>> my @bits=split(//,$str);
>> my @rnd=shuffle(@bits);
>> my $stringer="@rnd";
>> $stringer=~s/\s//g;
>> $stringer=substr($stringer,0,140);
>
> What Rainer said. I would add that their aren't any whitespace
> characters in your source, so their shouldn't be any in your output.
>


Justin.

--
Justin C, by the sea.

Jim Gibson <> writes:

[...]

>> my $str='1234567890!@#$
>> %^&*()qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJK LZXCVBNM_+{}|:"<>?~`-
>> =[]\;\',./1234567890!@#$
>> %^&*()qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJK LZXCVBNM_+{}|:"<>?~`-
>> =[]\;\',./';

[...]

> their aren't any whitespace characters in your source, so their
> shouldn't be any in your output.

There are: the newlines (\n).