OK, I havent really tried condesing someone else's code before, but one time has to be the first, right. As for the question you asked, I will need some more info on the general structure of your hash of arrays and filter array to give some specific advice, but you might want to take a look at map and grep.

Ok, lets have a look at your sub, first Build_Country_List. First do yourself a favor and use strict and warnings. You have one variable, $condition that I cant really see what does. Anyway, the sub seems to take 2 arguments, a boolean which is supposed to say if this is the first time the function is called, and a string that is to be inserted into the list if the country is not allready there.

The list seems to be some sort of global variable, so Ill keep it like that. usually not a good idea though, but for a throwaway script....

There are also lots of prints there, which I guess are debug statements. If so, I allways do something like this at the start of the module:

sub debug{
   print @_;
}
[download]

That way, the fact that it is debug statement are easy to see, and redirecting all to file, or removing them all can easily be done from one place.

There are actually 2 different ways I would have implemented your first sub, depending on requirements(speed vs memory usage):

This one uses some extra memory, but should be faster:

my %__privateIndexHash; #key = country read, value is allways 1
my @CountryList;
sub Build_Country_List($){
   #I do not really care about the boolean, Ill figure this out for my
+self.
   my $country=shift;
   if(!exists $__privateIndexHash{$country}){
   #Havent seen country before, insert into array and hash index
       $__privateIndexHash{country}=1;
       push @CountryList, $country;
   }
}
[download]

The second method does not use the hash index, but instead look trough the list each time.

my @CountryList;
Build_Country_List($){
   my $country=shift;
   push @CountryList, $country unless grep { $_ eq $country} @CountryL
+ist;
}
[download]

Did you get what I did here? Did I get what you tried to do? See if you understand what I did, and let me know if I misunderstood you. Feel free to ask questions while I study the next sub of yours.

Update Ok, here is a stab at the seond function. I have moved a little around on the if/else's and removed the foreach. Does this still do what you mean? I assume that $total is some sort of global variable.

my $total;
sub Build_totals_shares_for_sedol_Sell_side($$$) {
  my $sedol = shift;
  my $shares = shift;
  my $mySellfirstflag = shift;
  if ($mySellfirstflag eq "YES") {
    %Sellhash = ($sedol => $shares); 
    #Do you mean that with mySellfirst that we should replace the shar
+es,
    #If so, the above line should be $Sellhash{$sedol}=$shares.
    #As it stands now, we replace the entire hash!!
    print "Sell: The $sedol has traded\n$share\n";
  } else {
    if (exists $Sellhash{$sedol}) {
      print "the sedol is in the list so add the shares.\n";
      $Sellhash{"$sedol"} += $shares;
      my $newtotal = $Sellhash{"$sedol"};
      print "the new total shares for sedol: $sedol is $newtotal\n";
    } else {
      $Sellhash{$sedol} = $shares; #You did a .= here was that a typo?
      print "the new entry is $sedol $shares\n";  
    } 
    $total=0;
    $total += $_ foreach values %Sellhash;
  }
}
[download]

It can probably still be condensed some, but I think I need your feedback on some of the assumptions I have made first.

GoldClaw