Beefy Boxes and Bandwidth Generously Provided by pair Networks
Do you know where your variables are?
 
PerlMonks  

Comment on

( #3333=superdoc: print w/ replies, xml ) Need Help??
Update: Like I said, style's contentious. :-) Thanks to tye for interesting criticism on subroutine calls... read one of his posts for a well-thought-out opinion on the topic.


Update: Good call updating to make sure there are four octets, ybiC. I'm not bothering to update likewise. Rather than a counter, though, you can just check to see if @octets10 == 4.


I was going to do something cool for my 100th post, but I got tired of not posting while I worked on stuff. Oh, well. I never liked base-10 anyway. ;-)

I have come into the habit of abusing ybiC's posts in the ChatterBox via /msg... at ybiC's request. The conversation often deals with style as much as anything. These conversations, however, have outgrown the CB, so I'm posting this one, and rewriting the script according to my tastes. Some of it may be of general interest as well, but the style opinions are contentious.

Like 80 character lines. Darn it, ybiC, 75 Saves Lives! ;-)

Personally, I don't use & to denote subroutines except when I'm not supplying an argument list, and I want to make my current @_ available to the sub (check perlsub for details). So right off, s/&USAGE/USAGE/; I also don't much care for uppercase subroutine names, so I changed that too. What the heck.

It is not necessary to declare @octets8 and $octetpadded as global variables... they'll serve just fine as lexically scoped variables. Better not to thwart strict.

my(@octets8,$octetpadded);

This regex is somewhat less than optimal:

$octet10 =~ /\d{1,3}/

It matches octets which contain between 1 and 3 digits, but which may contain other characters as well. Thus when I enter:

./ipdec2oct.pl 10.m55.000.001

I get an incorrect octal address back:

octal 012.0.0.1

Since you've got warnings enabled, I also get a message complaining that

Argument "m55" isn't numeric in lt at ./ipdec2oct.pl line 22.

and hopefully that will lead me in the right direction as a user. But it's a problem.

Now interestingly, if you change your regex to match octets consisting solely of between one and three digits...

$octet10 =~ /^\d{1,3}$/

you not only take out the problem of extraneous characters, you prevent the possibility of an octet of less than zero. So you can remove the lines

if ( $octet10 < 0 ) { &USAGE("\"$address{decimal}\" don't cut it: \"$octet10\" is l +ess than 0"); }

On to your usage of USAGE. First off, let me point out the glory of qq(). You're using double quotes to contain your argument to &USAGE, because you need variables to interpolate within the argument... only problem is, you want double quotes within the argument. This leads to the frequent use of awkward escape sequences...

&USAGE("\"$address{decimal}\" don't cut it: \"$octet10\" is greater t +han 255");

Enter the qq operator. qq acts in the same way as double-quoting a string, and makes use of whatever delimiters you specify (q does likewise for single quoting). So this function call could be rewritten as

&USAGE(qq["$address{decimal}" don't cut it: "$octet10" is greater tha +n 255]);

or

&USAGE(qq."$address{decimal}" don't cut it: "$octet10" is greater tha +n 255.);

However, I'm not so sure that USAGE needs an argument. Why not just print your error message, and then call USAGE to print the usage message.

if ( $octet10 > 255 ) { print qq/"$address{decimal}" don't cut it: /, qq/"$octet10" is greater than 255./; &USAGE; }

As it turned out, the way I tweaked things qq() wasn't even needed; but it's still good to know.

Instead of saying

if ($octet8 > 7) { $octetpadded = "0$octet8"; } else { $octetpadded = $octet8; }

You could say

$octetpadded = ($octet8 > 7) ? "0$octet8" : $octet8;

which is more readable to me, at least.

For that matter, instead of saying

$octetpadded = ($octet8 > 7) ? "0$octet8" : $octet8; push @octets8, $octetpadded;

You could say

push @octets8, ($octet8 > 7) ? "0$octet8" : $octet8;

and eliminate the $octetpadded variable entirely. I don't know whether that's more readable, but it goes a long way towards explaining why I like Perl. ;-)

Looking at your foreach loop... a few things. First, as perhaps you know, foreach and for are synonyms. I mention this simply because it's worth mentioning, and I'm changing foreach to for simply because it's shorter and I prefer it.

for my $octet10(@octets10) {

Second, as perhaps you know as well, you could omit the my $octet10 part. If you do, the particular member referenced will be assigned to $_ instead. Whether you should do this in any particular circumstance is a matter of taste and debate. Here, it could be argued that the for(each) loop is long enough that using a more descriptive name enhances clarity. On the other hand, it could be argued that use of $_ reduces clutter, especially since there are no nested for(each) loops or other constructions which would assign to $_. Again, I'm going to opt for brevity, partially because that's how I'd do it, and partially because it's interesting to change things to see a different way.

for (@octets10) {

Instead of saying

my @octets10 = split /\./, $address{decimal}; for (@octets10) {

you have the option to say,

for (split /\./, $address{decimal}) {

You needn't, of course. I just like to because I'm a bit of a syntactic pervert and I'm into that sort of thing.

Likewise, I replaced

$address{octal} = join '.', @octets8; print "\n decimal $address{decimal}\n octal $address{octal}\n\n";

with

print "\n decimal ", $address{decimal}, "\n octal ", join('.',@octets8), "\n\n";

Of course, that means that there's no longer a $address{octal}, only an $address{decimal}... which means a hash is no longer useful. Dang it! Just after I'd finally gotten you using hashes. ;-)

To complete the nit-picking, let me point out that "aint" requires an apostrophe. ;-)

Anyway, with a few more modifications I've not mentioned, here's my stripped-down version. Not quite how I'd do it, but that's not the point; it's still your script at heart. Again, not all the changes are necessarily improvements.

#!/usr/bin/perl -w use strict; my @octets8; my $ip = shift; unless (defined $ip) { print 'Enter *something* for input'; usage(); } for (split /\./, $ip) { my $err = "$ip don't cut it: $_"; if ( !/^\d{1,3}$/ ) { print "$err ain't a number."; usage(); } elsif ( $_ > 255 ) { print "$err is greater than 255."; usage(); } else { my $oct = sprintf "%lo", $_; push @octets8, ($oct > 7) ? "0$oct" : $oct; } } print "\n decimal ", $ip, "\n octal ", join('.',@octets8), "\n\n"; sub usage { print join "\n ", "\n Usage: ipdec2oct.pl dottedquad_decimal_ipaddr\n", $0, "Perl $]", $^O, "\n"; exit; }

In reply to Re: IP address - decimal to octal by Petruchio
in thread (code) IP address - decimal to octal by ybiC

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post; it's "PerlMonks-approved HTML":



  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • Outside of code tags, you may need to use entities for some characters:
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.
  • Log In?
    Username:
    Password:

    What's my password?
    Create A New User
    Chatterbox?
    and the web crawler heard nothing...

    How do I use this? | Other CB clients
    Other Users?
    Others pondering the Monastery: (8)
    As of 2014-12-21 16:42 GMT
    Sections?
    Information?
    Find Nodes?
    Leftovers?
      Voting Booth?

      Is guessing a good strategy for surviving in the IT business?





      Results (106 votes), past polls