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

How can I keep object arguments private

by walto (Pilgrim)
on May 23, 2010 at 15:09 UTC ( #841260=perlquestion: print w/replies, xml ) Need Help??

walto has asked for the wisdom of the Perl Monks concerning the following question:

Fellow Monks,
I try to get a grip on objects in perl and there is a vast amount of questions coming up.
I wrote a module for retrieving data from a website. I call the object with a (only one at the moment) method and expect a hashref of the return value. But also the method options and object properties from the constructor are included in the return value. In order just to get the wanted values in the hashref I delete the values from the object before they are returned. But there will be problems when I call the method repeatedly.
Here is the code:
#!/usr/bin/perl # # package MeteoalarmCountries; use strict; use warnings; use Carp; use LWP; sub new { my $class = shift; my $self = {}; $self->{user_agent} = 'Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 5.1; Trident/4.0; GTB6; + .NET CLR 1.0.3705; .NET CLR 1.1.4322; Media Center PC 3.1; .NET CLR +2.0.50727; .NET CLR 3.0.4506.2152; .NET CLR 3.5.30729)'; bless( $self, $class ); return $self; } sub countries { my %warnpics = ( 'wf_21.jpg' => 'Wind 1', 'wf_22.jpg' => 'Snow/Ice 1', 'wf_23.jpg' => 'Thunderstorm 1', 'wf_24.jpg' => 'Fog 1', 'wf_25.jpg' => 'Extreme high temperature 1', 'wf_26.jpg' => 'Extreme low temperature 1', 'wf_27.jpg' => 'Coastal Event 1', 'wf_28.jpg' => 'Forestfire 1', 'wf_29.jpg' => 'Avalanches 1', 'wf_210.jpg' => 'Rain 1', ); my $self = shift; my %passed_params = @_; for my $parm ( keys %passed_params ) { $self->{$parm} = $passed_params{$parm}; }
my $url = 'http://www.meteoalarm.eu/index.php?lang=&AT=' . $self->{type} . '&day=' . $self->{day}; my $ua = LWP::UserAgent->new; $ua->agent( $self->{user_agent} ); my $res = $ua->request( HTTP::Request->new( GET => $url ) ); croak " $res->status_line, \n"unless ($res -> is_success); my $page = $res->content; my @splitted_pages = split '<a href=', $page; foreach my $splitted (@splitted_pages) { if ( $splitted =~ /id="(\w\w)" alt="(.+?)" ><span class="warn"><img src="Bilder\/wf\/wf_ +\d{2,3}.jpg">/ ) { my %warn; my $country_code = $1; my $country = $2; $self->{$country_code}->{'fullname'}= $country; my @warnings = $splitted =~ /<img src="Bilder\/wf\/(wf_\d{2,3}.jpg)">/g +; my @literal_warnings = map { $warnpics{$_} } @warnings; foreach my $warn (@literal_warnings) { my @part = split ' ', $warn; $warn{ lc $part[0] } = $part[1]; } $self->{$country_code}->{'warnings'} = \%warn; } } delete $self->{'user_agent'}; delete $self->{'day'}; delete $self-> {'type'}; return $self; }
Apparently I also need a hint with the readmore tag.

Replies are listed 'Best First'.
Re: How can I keep object arguments private
by toolic (Bishop) on May 23, 2010 at 16:53 UTC
    Apparently I also need a hint with the readmore tag.
    You can not embed readmore tags inside code tags. Place your readmore tags completely outside of your code tags.
    <readmore> <c> use strict; </c> </readmore>
      Thanks, that helped with the readmore tag.

      The other problem seems to be more a problem of the object design. With
      $self->{'countries'}->{$country_code}->{'fullname'}= $country; $self->{'countries'}->{$country_code}->{'warnings'} = \%warn; return $self->{'countries'};
      I am able to return just the wanted values without deleting any other properties of the object.
      Are there any other ways to return just the wanted value?
        Invoke laziness first: is there any reason I need to delete the other properties? Or will providing a wanted_value method (obviously a better name is needed!) suffice?
Re: How can I keep object arguments private
by FalseVinylShrub (Chaplain) on May 24, 2010 at 06:18 UTC

    Hi

    Well I'm not sure if you are asking about OO in general, or OO as implemented in Perl. Also I'm struggling to understand some parts of your code... Maybe if you could boil your code down to a simple example that illustrates your question...

    Anyway, here are some observations to start with (just stating the obvious in case I've misunderstood something):

    • You've a class called MeteoalarmCountries
    • new() sets a property for the user agent
    • The method countries:
      1. Keeps a lookup of filenames for images
      2. Retrieves $self and stores the rest of the parameters in $self as hash keys
      3. Builds a URL and fetches it with LWP
      4. splits the result on <a href=
      5. Checks each element of the split for some ID codes and stores something in $self based on looking up in the image hash
      6. deletes the keys user_agent, day and type from $self
      7. returns $self
    • You later mention that by storing the country codes in $self->{countries} you can distinguish them from the parameters and avoid the need to delete them.

    Now for some commentary:

    You are correct that you can differentiate between the country data and other things by using a nested structure. However, why are you storing the method parameters and method results in the object? The object's properties should be things needed between calls to the method. The use of user_agent seems appropriate, though later you may want to create get/set method(s) for it.

    Why not just store the method parameters in local variables or a hash?

    The same for the return value: if it's a list or hash just build it into a local variable and just return it. If that information is required for later method calls, storing it in the object is appropriate, but the code would be clearer with a get/set method in my view.

    Sometimes we return $self to allow method calls to be chained (e.g. $self->do_something->do_something_else->etc). However you don't need to return $self because the caller already has it. In your case, the method is called countries so I think it should return some countries.

    Here's some pseudo-Perl illustrating how I think you could structure it:

    # pseudo-Perl only, not for executing Package MeteoAlarmCountries; # ... use stuff... sub new { # ... } sub fetch_country_warnings { my $self = shift; my %p = @_; my $url = _make_url($p{day}, $p{type}); my $content = _fetch_content($url, $self->get_user_agent); my $warnings = _extract_country_warnings($content); return $warnings; } sub _make_url { # ... } sub _fetch_content { # ... } sub get_user_agent { # return static default, # or do a config lookup, # or allow get/set from outside return $ua_string; } sub _extract_country_warnings { my $content = shift; # ... my %lookup_table = ( ... ) my %country_warnings; foreach ( ... split ... ) { # match and lookup $country_warnings{$country} = $something } return \$country_warnings; }

    If the countries/warnings are required to be stored in the object for later, you could split fetch_country_warnings into fetch_and_store_country_warnings and a simple get/set method country_warnings

    I hope this makes sense...

    By the way, what happens if the required tag is written <a id="blah" href="whatever">etc</a>? It won't match your split.

    FalseVinylShrub

    Disclaimer: Please review and test code, and use at your own risk... If I answer a question, I would like to hear if and how you solved your problem.

      Thanks a lot FalseVinylShrub and ikegami for your detailed answers!!

      There is no need to keep the method options in the object. So, as you pointed, it is a good idea to keep them in a variable. Since I want to distinguish between fullname and warnings I store this values in a hash
      $country_warnings->{$country_code}->{'fullname'} =$country; $country_warnings->{$country_code}->{'warnings'} = \%warn; return $country_warnings;
      Doing this gets me a return value that I want: MeteoalarmCountry::countries->{fullname} and ...:countries->{warnings}. I have no need to keep this values in the object.
      And you are perfectly right it is also a good idea to check if $country_code is defined before assigning values to this key.
Re: How can I keep object arguments private
by ikegami (Pope) on May 24, 2010 at 06:21 UTC

    and expect a hashref of the return value.

    Then what are you doing returning an object ($self)? I'm not clear on what your object is suppose to do. Tell us about your object and what the referenced hash should contain and we'll help you further.

    After looking some more, I think my approach would be something like

    { package Meteoalarm; sub new { my ($class, %args) = @_; my $self = bless({}, $class); $self->{ua} = _make_ua(); return $self; } sub _make_ua { return LWP::UserAgent->new( ... ); } sub get_warnings { my ($self) = @_; my $ua = $self->{ua}; ... my %by_country; for (...) { ... $by_country{$country} = \%warning; } return \%by_country; } }
    or
    { package Meteoalarm; sub new { my ($class, %args) = @_; my $self = bless({}, $class); $self->{ua} = _make_ua(); return $self; } sub _make_ua { return LWP::UserAgent->new( ... ); } sub fetch { my ($self) = @_; my $ua = $self->{ua}; ... my @countries; for (...) { ... push @countries, Meteoalarm::Country->new( name => $country, warnings => \%warnings, ); } return \@countries; } } { package Meteoalarm::Country; sub new { my ($class, %args) = @_; my $self = bless({}, $class); $self->{name } = $args{name }; $self->{warnings} = $args{warnings}; return $self; } sub name { shift->{name } } sub warnings { shift->{warnings} } }
    Or maybe something else. It really depends on how you plan on using the objects.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://841260]
Approved by johngg
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others exploiting the Monastery: (4)
As of 2021-01-23 21:04 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    Notices?