http://qs321.pair.com?node_id=30417


in reply to Win32::MemoryInfo

Cool.

One things I hate about Win32::API is its encouragement of creating strings from barewords. Drop the no strict "subs" and just do:

my $GlobalMemoryStatus ||= Win32::API->new( "kernel32", "GlobalMemoryStatus", ["P"], "V" ) or return;

I would not export anything nor have a separate routine and simply pass in the format to the routine as "B" (or nothing) for bytes, "K" for 1024, "M" for 1024*1024, or "G" for 1024**3. I would just drop the whole ceil stuff. What is wrong with saying you have 2.4MB of memory available? And put the single remaining exportable symbol into @EXPORT_OK not @EXPORT.

And the only major change: You have enough parameters being returned that I'd return them as a hash reference so I don't have to keep looking up what order those values come back in. For extra credit, let the user specify a list of parameters that they are interested in. Return a list of just those values or return the hash ref if no particular parameters were requested.

        - tye (but my friends call me "Tye")

Replies are listed 'Best First'.
(Guildenstern) REx2: Win32::MemoryInfo
by Guildenstern (Deacon) on Aug 31, 2000 at 17:51 UTC
    Whew! Sounds like I know what I'm doing this morning. All great suggestions, and they should be pretty easy to implement.
    I do have one question about your suggestion of letting the user specify what values are wanted back. The only thing I can think of off the top of my head is to have the user fill a hash with certain values, i.e. "TotalPhysMem" => 0, "AvailPhysMem" => 0, etc. Then have the function only fill in the values that are present in the hash. How do I get the hash to the function? Do I need to change the prototype from
    @returns = MemoryInfo();
    to
    MemoryInfo(%returns,"M");
    ?

    Guildenstern
    Negaterd character class uber alles!

      I'll start from the beginning because I don't think I'll make much sense otherwise.

      Okay, the basic idea is that, with so many values being returned (3 is usually too many in my book), remembering the exact order of the values is problematic. Also, if you are returning a whole list of things, it'd be nice to design the interface so it can evolve in an intelligent manner.

      So, just like when you have lots of parameters, it is nice to "name" them (func(This=>"that",Count=>2)), you can return "named" values by returning a hash. I try to avoid returning hashes as lists so let's return a reference to a hash.

      One of the nice things about named parameters is that you can leave out any parameters that you aren't interested in. So let's allow the user the same option here. If they only want page file information in megabytes, they can write:

      my( $avail, $total )= MemoryStatus( "MB", "AvailPageFile", "TotalPageFile" );

      The "MB" parameter is optional and we can detect it due to it being different from any of the field names. If you think you may further extend the interface in the future, you might want to make this distinction clearer and allow for extra arguments by having the field names passed as an array reference:

      my( $avail, $total )= MemoryStatus( "MB", [ "AvailPageFile", "TotalPageFile" ] );
      but I don't think that is warranted in this case.

      If the user wants all or most of the items, then they use it like this:

      my $hMem= MemoryStatus( "K" ); my $load= $hMem->{MemoryLoad};

      This is particularly handy for add-hoc queries in the Perl debugger:

      perl -de 0 DB<1> use Win32::MemoryInfo "MemoryStatus" DB<2> x MemoryStatus() 0 HASH(0x1df2628) 'AvailPageFile' => 3026 'TotalPageFile' => 32768 [...]

      And Perl makes this easy to code as well:

      my @fieldNames= qw( MemoryLoad TotalPhys AvailPhys TotalPageFile AvailPageFile TotalVirtual AvailVirtual ); my %unitSize= ( B=>1, K=>1024, M=>1024*1024, G=>1024*1024*1024 ); #[...] my( $sUnits )= "B"; if( @_ && $_[0] =~ /^[BKMG]B?$/i ) { $sUnits= uc substr(shift(@_),0,1); } my( $nUnits )= $unitSize{$sUnits}; my( $dwMSLength )= 0; my( @fieldValues )= (0)x@fieldNames; my $MEMORYSTATUS= pack "L8", $dwMSLength, @fieldValues; $GlobalMemoryStatus->Call($MEMORYSTATUS); ( $dwMSLength, @fieldValues )= unpack "L8", $MEMORYSTATUS; return if 0 == $dwMSLength; for( @fieldValues ) { $_ /= $nUnits; } my $hFields= {}; @$hFields{@fieldNames}= @fieldValues; return @_ ? @$hFields{@_} : $hFields;
              - tye (but my friends call me "Tye")
        Okay. I see what you're saying about the parameters now. I was thinking of doing something like this:
        my %meminfo = { TotalPhys=>0, AvailPhys=>0 }; MemoryStatus(\%meminfo,"KB");

        but it seemed like too much work to make the user do, and setting hash values to 0 might be confusing. It would lessen the task of checking the parameter list, though, and we still have a hash reference which I agree is better than returning a specifically ordered list. Maybe I can include both options?

        Guildenstern
        Negaterd character class uber alles!
        Here's what I came up with after some reworking. I decided to go with a hash reference as the main parameter instead of taking words. This also means that it must now be called like this:
        my %hMem = (TotalPhys =>0, AvailPhys =>0); Win32::MemoryInfo::MemoryStatus(%hMem,"MB");

        I don't know what the consensus is on putting values into a hash like that, but if someone doesn't check the return value at least they won't get bit with undef values.
        Here's the reworked function:
        sub MemoryStatus (\%;$) { my $return = shift; my $ret_type = shift; my %fmt_types = ( B => 1, KB => 1024, MB => 1024*1024, GB => 1024*1024*1024); my @params = qw(MemLoad TotalPhys AvailPhys TotalPage AvailPage TotalVirtual AvailVirtual); my %results; my $MemFormat; my $dwMSLength; $MemFormat = ($ret_type =~ /^[BKMG]B?$/) ? $fmt_types{$ret_type} : $fmt_types{B} +; my $GlobalMemoryStatus ||= new Win32::API("kernel32", "GlobalMemoryStatus", ["P"], "V") or ret +urn undef; my $MEMORYSTATUS = pack "L8",(0, 0, 0, 0, 0, 0, 0, 0); $GlobalMemoryStatus->Call($MEMORYSTATUS); ($dwMSLength, @results{@params}) = unpack "L8", $MEMORYSTATUS; return undef if ($dwMSLength == 0); if (keys(%$return) == 0) { foreach (@params) { $return->{$_} = ($_ eq "MemLoad") ? $results{$_} : $results{$_}/$M +emFormat; } } else { foreach (@params){ $return->{$_} = $results{$_}/$MemFormat unless (!defined($return- +>{$_})); } } 1; }


        Guildenstern
        Negaterd character class uber alles!