School Assignment? Project Due Tomorrow? Chat LIVE With A Programming Expert!

Programming Help

Web Development



What's Here?

  • Members: 300,365
  • Replies: 825,881
  • Topics: 137,435
  • Snippets: 4,419
  • Tutorials: 1,147
  • Total Online: 1,438
  • Members: 104
  • Guests: 1,334

Welcome to Dream.In.Code
Become an Expert!

Join 300,365 Programmers for FREE! Get instant access to thousands of experts, tutorials, code snippets, and more! There are 1,438 people online right now. Registration is fast and FREE... Join Now!




Looking for smart solution

 

Looking for smart solution, reading file checking record with hash!!!
Sun751

23 Jun, 2009 - 01:12 AM
Post #1

D.I.C Head
**

Joined: 11 Dec, 2008
Posts: 57



Thanked: 1 times
My Contributions
In my project I am reading the XML file and writing into new file updating few data in it, for which I have used hash to check if record exist, I have written the code and its working well, but i was think if there is any smart solution I mean less code to do same task!!!! here is my code:-

CODE

sub dispatch
{
    my $HRR_resource = shift;
    my $HRR_wnt = shift;
    my $HRR_unx = shift;
    my $HRR_Scomponent = shift;
    my $HRR_Wcomponent = shift;
    my $Pkey;
    my $Wkey;
    my $Ukey;
    my $SCkey;
    my $WCkey;
    my $Flag;

    open(FR, "<$source_file")|| die "Unable to open input file: $sourc
+e_file $!";
    open(FW, ">$dest_file")|| die "Unable to open output file: $dest_f
+ile $!";

    while (my $line=<FR>)
    {
    $line =~tr/\r\n//d;
    if ($line)
    {
        $Flag = 1;
        if ($line =~ /PLATFORM_INDEPENDENT/){$Pkey = $line};
        if ($Pkey)
        {
            $Flag = undef;
            $Pkey =~ s/\s*//;
            if($Pkey ne '</PLATFORM_INDEPENDENT>')
            {
               if ($line =~ /Id="(.*?)".*?>(.*?)</)
               {
                   my $key_id = $1;
                   my $key_val = $2;

                   if (exists $$HRR_resource{$key_id})
                   {
                        my $val = $$HRR_resource{$key_id};
                        $val =~ /.*?>(.*?$)/;
                        $val = $1;
                        $line =~ s/>.*?</>$val</;
                        print FW "$line\n";
                        delete($$HRR_resource{$key_id});
                   }
                   else
                   {
                        print FW "$line\n";
                   }
               }
               else
               {
                   print FW "$line\n";
               }
            }
            if ($line =~ /<\/PLATFORM_INDEPENDENT>/)
            {
                foreach my $addline(keys %$HRR_resource)
                {
                   print FW  "<RESOURCE Id=\"$addline\" $$HRR_resource
+{$addline}</RESOURCE>}\n";
                }
                print FW "$line\n";
                $Pkey = undef;
            }
        }
        if ($line =~ /WNT/){$Wkey = $line};
        if ($Wkey)
        {
            $Flag = undef;
            $Wkey =~ s/\s*//;
            if ($Wkey eq '<WNT>' && $Wkey ne '</WNT>')
            {
               if ($line =~ /Id="(.*?)".*?>(.*?)</)
               {
                   my $key_id = $1;
                   my $key_val= $2;
                   if (exists $$HRR_wnt{$key_id})
                   {
                        my $val = $$HRR_wnt{$key_id};
                        $val =~ /.*?>(.*?$)/;
                        $val = $1;
                        $line =~ s/>.*?</>$val</;
                        print FW "$line\n";
                        delete($$HRR_wnt{$key_id});
                   }
                   else
                   {
                        print FW "$line\n";
                   }
               }
               else
               {
                   print FW "$line\n";
               }
            }
            if ($line =~ /<\/WNT>/)
            {
                foreach my $addline(keys %$HRR_wnt)
                {
                   print FW  "<RESOURCE Id=\"$addline\" $$HRR_wnt{$add
+line}</RESOURCE>}\n";
                }
                print FW "$line\n";
                $Wkey = undef;
            }
        }
        if ($line =~ /UNX/){$Ukey = $line};
        if ($Ukey)
        {
            $Flag = undef;
            $Ukey =~ s/\s*//;
            if ($Ukey ne '</UNX>')
            {
               if ($line =~ /Id="(.*?)".*?>(.*?)</)
               {
                   my $key_id = $1;
                   my $key_val= $2;
                   if (exists $$HRR_unx{$key_id})
                   {
                        my $val = $$HRR_unx{$key_id};
                        $val =~ /.*?>(.*?$)/;
                        $val = $1;
                        $line =~ s/>.*?</>$val</;
                        print FW "$line\n";
                        delete($$HRR_unx{$key_id});
                   }
                   else
                   {
                        print FW "$line\n";
                   }
               }
               else
               {
                   print FW "$line\n";
               }
            }
            if ($line =~ /<\/UNX>/)
            {
                foreach my $addline(keys %$HRR_unx)
                {
                   print FW  "<RESOURCE Id=\"$addline\" $$HRR_unx{$add
+line}</RESOURCE>}\n";
                }
                print FW "$line\n";
                $Ukey = undef;
            }
        }
if ($Flag)
         {
            print FW "$line\n";
         }
      }
   }
}


Any Suggestion???
Cheers!!!

User is offlineProfile CardPM
+Quote Post


dsherohman

RE: Looking For Smart Solution

23 Jun, 2009 - 05:12 AM
Post #2

D.I.C Head
**

Joined: 29 Mar, 2009
Posts: 184



Thanked: 35 times
My Contributions
QUOTE(Sun751 @ 23 Jun, 2009 - 09:12 AM) *

In my project I am reading the XML file and writing into new file updating few data in it, for which I have used hash to check if record exist, I have written the code and its working well, but i was think if there is any smart solution I mean less code to do same task!!!!


This one is too long for me to easily wrap my head around it, so I won't do a full refactoring this time, but that leads directly into my first suggestion: Break the code up into smaller subroutines with each do a single conceptual task. It might get a little longer, but so what? It will be much, much easier to read and to maintain.

CODE

sub dispatch
{
    my $HRR_resource = shift;
    my $HRR_wnt = shift;
    my $HRR_unx = shift;
    my $HRR_Scomponent = shift;
    my $HRR_Wcomponent = shift;


This can be tightened up considerably by using list assignment instead of shift:
CODE

    my ($HRR_resource, $HRR_wnt, $HRR_unx,
              $HRR_Scomponent, $HRR_Wcomponent) = @_;


CODE

    my $Pkey;
    my $Wkey;
    my $Ukey;
    my $SCkey;
    my $WCkey;
    my $Flag;


Similarly, you can use
CODE

    my ($Pkey, $Wkey, $Ukey, $SCkey, $WCkey, $Flag);

here.

CODE

    open(FR, "<$source_file")|| die "Unable to open input file: $source_file $!";
    open(FW, ">$dest_file")|| die "Unable to open output file: $dest_file $!";


You should use the three-argument form of open rather than the two-argument form, as it will help to protect you against certain classes of errors and security vulnerabilities:
CODE

    open(FR, '<', $source_file)|| die "Unable to open input file: $source_file $!";
    open(FW, '>', $dest_file")|| die "Unable to open output file: $dest_file $!";


It's also currently recommended to use lexical filehandles (e.g., open(my $fr, '<', $source_file)...) instead of typeglob filehandles (FR), but that's less of an issue.

CODE

    if ($line)
    {


Conditional clauses spanning multiple pages are bad news, as they obscure the flow of control. Moving the huge blocks of processing out into separate subroutines will help considerably with this, but, if you're doing a bunch of processing if $line is true and just going on to the next iteration of your loop if it's false, it's much cleaner and tighter to just use
CODE

        next unless $line;


CODE

$val =~ /.*?>(.*?$)/;
$val = $1;
$line =~ s/>.*?</>$val</;


I strongly advise taking a look at CPAN ( http://search.cpan.org ) to find an appropriate module to handle the XML parsing and manipulation for you. Parsing XML using your own hand-rolled regexes is extremely difficult to get right. (Yes, I know you said your current code works with the data you've given it so far. XML contains plenty of corner cases which will eventually trip you up and cause much wailing and gnashing of teeth if your hand-rolled code proves successful enough that it one day needs to deal with data from other sources.)

As for the rest, pulling it out into subroutines, replacing the XML parsing with an appropriate CPAN module, and applying the techniques I've shown you in my previous replies should go a long way towards taming this code.
User is offlineProfile CardPM
+Quote Post

Sun751

RE: Looking For Smart Solution

23 Jun, 2009 - 04:18 PM
Post #3

D.I.C Head
**

Joined: 11 Dec, 2008
Posts: 57



Thanked: 1 times
My Contributions
QUOTE(dsherohman @ 23 Jun, 2009 - 05:12 AM) *

QUOTE(Sun751 @ 23 Jun, 2009 - 09:12 AM) *

In my project I am reading the XML file and writing into new file updating few data in it, for which I have used hash to check if record exist, I have written the code and its working well, but i was think if there is any smart solution I mean less code to do same task!!!!


This one is too long for me to easily wrap my head around it, so I won't do a full refactoring this time, but that leads directly into my first suggestion: Break the code up into smaller subroutines with each do a single conceptual task. It might get a little longer, but so what? It will be much, much easier to read and to maintain.

CODE

sub dispatch
{
    my $HRR_resource = shift;
    my $HRR_wnt = shift;
    my $HRR_unx = shift;
    my $HRR_Scomponent = shift;
    my $HRR_Wcomponent = shift;


This can be tightened up considerably by using list assignment instead of shift:
CODE

    my ($HRR_resource, $HRR_wnt, $HRR_unx,
              $HRR_Scomponent, $HRR_Wcomponent) = @_;


CODE

    my $Pkey;
    my $Wkey;
    my $Ukey;
    my $SCkey;
    my $WCkey;
    my $Flag;


Similarly, you can use
CODE

    my ($Pkey, $Wkey, $Ukey, $SCkey, $WCkey, $Flag);

here.

CODE

    open(FR, "<$source_file")|| die "Unable to open input file: $source_file $!";
    open(FW, ">$dest_file")|| die "Unable to open output file: $dest_file $!";


You should use the three-argument form of open rather than the two-argument form, as it will help to protect you against certain classes of errors and security vulnerabilities:
CODE

    open(FR, '<', $source_file)|| die "Unable to open input file: $source_file $!";
    open(FW, '>', $dest_file")|| die "Unable to open output file: $dest_file $!";


It's also currently recommended to use lexical filehandles (e.g., open(my $fr, '<', $source_file)...) instead of typeglob filehandles (FR), but that's less of an issue.

CODE

    if ($line)
    {


Conditional clauses spanning multiple pages are bad news, as they obscure the flow of control. Moving the huge blocks of processing out into separate subroutines will help considerably with this, but, if you're doing a bunch of processing if $line is true and just going on to the next iteration of your loop if it's false, it's much cleaner and tighter to just use
CODE

        next unless $line;


CODE

$val =~ /.*?>(.*?$)/;
$val = $1;
$line =~ s/>.*?</>$val</;


I strongly advise taking a look at CPAN ( http://search.cpan.org ) to find an appropriate module to handle the XML parsing and manipulation for you. Parsing XML using your own hand-rolled regexes is extremely difficult to get right. (Yes, I know you said your current code works with the data you've given it so far. XML contains plenty of corner cases which will eventually trip you up and cause much wailing and gnashing of teeth if your hand-rolled code proves successful enough that it one day needs to deal with data from other sources.)

As for the rest, pulling it out into subroutines, replacing the XML parsing with an appropriate CPAN module, and applying the techniques I've shown you in my previous replies should go a long way towards taming this code.



Thanks dsherohman for your comments, highly appreciate it. It is true that to update xml file its better and easy to use perl xml module specially (XML::Twig and LibXML) and I highly recommend it for any one interested.

Unfortunately, I am not using any of these modules because I don’t have the right to install it and system admin not interested to do so!!!

Cheers
User is offlineProfile CardPM
+Quote Post

chorny_cpan

RE: Looking For Smart Solution

25 Jun, 2009 - 02:55 AM
Post #4

New D.I.C Head
Group Icon

Joined: 13 May, 2009
Posts: 35


Dream Kudos: 25
My Contributions
QUOTE(Sun751 @ 23 Jun, 2009 - 04:18 PM) *

Unfortunately, I am not using any of these modules because I don’t have the right to install it and system admin not interested to do so!!!


See Of course you can use CPAN. And here's why.. Also there are pure-perl XML parsing modules that you can bundle with your program.
User is offlineProfile CardPM
+Quote Post

Fast ReplyReply to this topicStart new topic

Time is now: 11/7/09 08:44PM
Invision Power Board v2.1.7 © 2009  IPS, Inc.
Licensed to: MediaGroup1

Live Help!

Be Social

Dream.In.Code RSS Feed Dream.In.Code LinkedIn Group Follow Us On Twitter Fan Us On Facebook

Tutorials

Programming

Web Development

Reference Sheets

Code Snippets

DIC Chatroom

Bye Bye Ads

Monthly Drawing

Thumb Drive

Top Contributors

Top 10 Kudos This Month