Beefy Boxes and Bandwidth Generously Provided by pair Networks
Perl: the Markov chain saw
 
PerlMonks  

comment on

( [id://3333]=superdoc: print w/replies, xml ) Need Help??

malloc asked for comments about his style/coding practices, and I saw his useful netswap.pl program as a good exercise for a code/style review (not golf!).
I'm not claiming that the proposed code is perfect nor the original code is bad (in fact, I really like the use of the -i switch to edit files in a convenient way in a program), but I hope to give some hints on how can one use perlish features to improve code.

  • Use strict and -w are personnaly a must when I have to write programs that are either more than 10 lines long or are to be used more than 10 times (I guess I first read that in one of merlyn's column)

    #!/usr/bin/perl -wi.bak use strict;
  • I replaced the bunch of scalars by a hash. It will be easier to add other keys if we have to process other files in the future.
    Also note that you could have declared several my variables with something like "my ($var1, $var2, $var3);".

    my %params; my $filename = shift || die "Usage: netswap.pl CONFILENAME\n";
  • I added a meaningful error message by appending the "$!" variable, to indicate precisely what the problem is.

    open(FH,"$filename") or die "Could not open network configuration file: [$filename] for + reading: $!\n";
  • I got rid of the "$line = $_", as the $line scalar was not really useful for future operations.
    The key/value extracted are stored in a hash, which is I think the easiest and safest way to do.
    Using "soft references" as you did (${$param_name} = $param_val;) might cause some problems (for instance, what happens if you extract a key named 'value'?).

    while(<FH>){ # Skip the blank lines or the commented lines next if (/^\s*$/ || /^\s*\#/); # Get the name and the values my ($name, $value) = split(/:/, $_, 2); # Remove starting/trailing spaces s/(?:^\s*|\s*$)//g foreach ($name, $value); # Store the values in the hash in lowercase to # avoid typing mistakes $params{$name} = lc($value); }
  • Mostly cosmetic choice here, I did a here-document instead of printing a several lines long string. I just happen to find here-docs more readable.

    print <<"EOL"; new hostname = [$params{hostname}] new domainname = [$params{domainname}] new ip = [$params{ip}] new Subnet Mask = [$params{subnetmask}] new DNS = [$params{dns}] new gateway = [$params{gateway}] EOL
  • Now, here's maybe my most criticizable modification. Instead of calling several times your file_edit function, I stuffed a big hash reference with all the infos on the replacements to occur in the file.
    The filenames are the keys of a second hash with the regex to modify as key and the modified string as value.

    my $replacements = { '/etc/hosts' => { 'hosts' => { "^.*$params{hostname}.*" => "$params{ip}\t$params{hostname}.$p +arams{domainname}\t$params{hostname}", }, '/etc/sysconfig/network' => { "^GATEWAY.*" => qq'GATEWAY="$params{gateway}"', "^HOSTNAME.*" => qq'HOSTNAME="$params{hostname}"', "^DOMAINNAME.*" => qq'DOMAINNAME="$params{domainname}"', }, '/etc/sysconfig/network-scripts/ifcfg-eth0' => { "^IPADDR.*" => qq'IPADDR="$params{ip}"', "^NETMASK.*" => qq'NETMASK="$params{subnetmask}"', }, '/etc/resolv.conf' => { "^nameserver.*" => "nameserver $params{dns}", "^SEARCH.*" => "SEARCH $params{domainname}", }, }; file_edit($replacements); `/etc/init.d/network restart`; ########################file_edit################### sub file_edit { my $replacements = shift; foreach my $filename (keys %{$replacements}){ print STDERR "Processing file [$filename]\n";
  • Instead of harshly dying, I chose print an error and skip to the next file, if the file doesn't exists.

    unless (-e $filename){ print STDERR " [$filename] cannot be found!\n"; next; }
  • I really like the use of the -i switch to modify inplace the files! However I added the .bak suffix, just in case something goes wrong.

    @ARGV=($filename); # put the filename where perl can find it. while(<>){ foreach my $search (keys %{$replacements->{$filename}}){ s/$search/$replacements->{$filename}->{$search}/; print; } } print STDERR " done\n"; } }

    For better advices and hints on how to improve one's code and style, I strongly suggest reading Dominus most excellent "Program Repair Shop and Red Flags" series available at Dominus' homepage.

    Now, I'd really like to get some coding/style review on the code above!

    If everything goes well, you should get the whole new code in one piece if you press d/l code!

    <kbd>--
    my $OeufMayo = new PerlMonger::Paris({http => 'paris.mongueurs.net'});</kbd>

In reply to Re: netswap.pl (style/coding comments) by OeufMayo
in thread netswap.pl by malloc

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



  • Are you posting in the right place? Check out Where do I post X? to know for sure.
  • Posts may use any of the Perl Monks Approved HTML tags. Currently these include the following:
    <code> <a> <b> <big> <blockquote> <br /> <dd> <dl> <dt> <em> <font> <h1> <h2> <h3> <h4> <h5> <h6> <hr /> <i> <li> <nbsp> <ol> <p> <small> <strike> <strong> <sub> <sup> <table> <td> <th> <tr> <tt> <u> <ul>
  • Snippets of code should be wrapped in <code> tags not <pre> tags. In fact, <pre> tags should generally be avoided. If they must be used, extreme care should be taken to ensure that their contents do not have long lines (<70 chars), in order to prevent horizontal scrolling (and possible janitor intervention).
  • Want more info? How to link or How to display code and escape characters are good places to start.
Log In?
Username:
Password:

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

How do I use this?Last hourOther CB clients
Other Users?
Others drinking their drinks and smoking their pipes about the Monastery: (8)
As of 2024-04-18 15:00 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found