Beefy Boxes and Bandwidth Generously Provided by pair Networks
good chemistry is complicated,
and a little bit messy -LW
 
PerlMonks  

Re: How do I go from procedural to object oriented programming?

by choroba (Cardinal)
on Apr 20, 2015 at 22:06 UTC ( [id://1124075]=note: print w/replies, xml ) Need Help??


in reply to How do I go from procedural to object oriented programming?

Your code is indeed very close to Object Oriented. I translated part of it to Moo:
#!/usr/bin/perl use strict; use warnings; my %movies_data = ( 'Firefly' => { 'start year' => 2002, 'end year' => 2003, 'media' => 'tv', }, 'Criminal Minds' => { 'start year' => 2005, 'end year' => 'tbd', 'media' => 'tv', }, 'The 10th Kingdom' => { 'start year' => 2000, 'media' => 'miniseries', }, 'Iron Man' => { 'start year' => 2008, 'media' => 'film', 'based on' => 'comics', 'company' => 'Marvel Comics', }, 'Tin Man' => { 'start year' => 2007, 'media' => 'miniseries', 'based on' => 'novel', 'company' => 'L. Frank Baum' }, 'The Avengers (1998)' => { 'start year' => 1998, 'media' => 'film', 'based on' => 'television + series', 'company' => 'Thames Tele +vision', }, ); { package Movie; use Moo; use Time::Piece; my $current_year = localtime->year; has 'title' => ( is => 'ro', required => 1 ); has 'media' => ( is => 'ro', required => 1, isa => sub { shift =~ /^(?:tv|miniseries|film)$/ or d +ie } ); has 'start_year' => ( is => 'ro', required => 1, isa => sub { shift > 1870 or die } ); has 'end_year' => ( is => 'ro', default => $current_year ); has 'based_on' => ( is => 'ro', isa => sub { shift =~ /^(?:comics|novel|television se +ries)$/ or die }); has 'company' => ( is => 'ro' ); around media => sub { my ($orig, $self) = @_; my $media = $self->$orig; return 'tv' eq $media ? 'television series' : $media }; sub movie_is { my $self = shift; my $movie_is = join ' ', $self->title, 'is a', grep defined, $self->start_year, $self->media, $self->basis, $self->run_time; return "$movie_is." } sub basis { my $self = shift; my $basis = $self->based_on ? ('based on the ' . $self->based_on . ' by ' . $self->compa +ny) : undef; return $basis } sub run_time { my $self = shift; my $run_text; if ('television series' eq $self->media) { if ($self->end_year eq 'tbd') { $run_text = 'which is still running'; } elsif ((my $run_time = $self->end_year - $self->start_ye +ar) > 0) { $run_text = "which ran for $run_time year" . ($run_time > 1 ? 's' : q()); } } return $run_text } } for my $title (sort keys %movies_data) { my $movie = 'Movie'->new( title => $title, map { (my $t = $_) =~ s/ /_/; $t => $movies_data{$title}{$_} } keys %{ $movies_data{$title} }); print $movie->movie_is, "\n"; }

I don't like the result much, though. When writing OOP code, you should concentrate on methods (what the objects do), not on the attributes (what properties the objects have). I haven't studied the full code to find out more than shown here, but e.g. the run_time method seems ugly.

Update: Added isa checks.

لսႽ† ᥲᥒ⚪⟊Ⴙᘓᖇ Ꮅᘓᖇ⎱ Ⴙᥲ𝇋ƙᘓᖇ

Replies are listed 'Best First'.
Re^2: How do I go from procedural to object oriented programming?
by Lady_Aleena (Priest) on Apr 22, 2015 at 07:37 UTC

    Hello choroba and thank you for stopping by. I have a few specific questions about the code you provided. I will be writing up something more general as a reply to my OP to show the true breadth of the module.

    1. How are you getting %movies_data from the module to the script below?
      #!/usr/bin/perl use strict; use warnings; # Movie is the only package name I found in the module, # so I'm guessing it is also the module's name. use Movie; for my $title (sort keys %movies_data) { my $movie = 'Movie'->new( title => $title, map { (my $t = $_) =~ s/ /_/; $t => $movies_data{$title}{$_} } keys %{ $movies_data{$title} }); print $movie->movie_is, "\n"; }
      I see %movies_data but I don't see how you got it from the module. %movies_data is not something I want to have to copy and paste to the three scripts which use it now.
    2. Why did you constrain media and based_on? media could grow to include web series, music videos, and even video games (interactive movies). based_on includes so much more than the few things I included in the sample data. There are movies based on video games, board games, role-playing games, plays, etc. I'm waiting for the SyFy Original movie Qwerty vs. Dvorak which would be based on computer equipment without a company.
    3. How would your code handle movies with the start year "tbd"?
    4. It is easy to strip off the Moo formatting to make it normal OO? My web host does not have Moo installed.
    5. Does this code work in perl 5.8.8?

    A side note: run_time is not as bad as some of the other subroutines in the module which are truly hairy.

    No matter how hysterical I get, my problems are not time sensitive. So, relax, have a cookie, and a very nice day!
    Lady Aleena
      1. My example was not a module, it was a script with an embedded package. package has a block scope, so to create a real module, only save the portion of the script that defines the package. The rest (including the %movies_data hash) belongs to the main package, i.e. to the script.
      2. I prefer to constrain values. You can add any values you need, but checking they belong to a given set can still help you: it will catch typos in newly added data and provide a list of all possible values when you're not sure what category to use.
      3. The same way as your code, i.e. there's no particular code related to the case (maybe because there's no such movie in the data?).
      4. It's possible. It can take some time, but it isn't hard.
      5. I haven't tested it, but I think so. I'm not aware of any 5.10+ features used in the code.
      لսႽ† ᥲᥒ⚪⟊Ⴙᘓᖇ Ꮅᘓᖇ⎱ Ⴙᥲ𝇋ƙᘓᖇ
        1. Sorry, I thought you wrote a module and script. I did not realize it was only a script. It would be very painful to have to copy and paste almost everything from line 8 to line 213 (nearly 6kb) in the current module to every script which uses the 5 hash subroutines the module currently exports. Oh guh...
        2. I had not thought about setting things up within the subroutines to check for spelling errors. I usually do that with a scratchpad script to find things which do not not belong.
        3. *blushes* I did not think to add a movie with a tbd start date.
        4. So Moo has the path from regular OO to Moo in the docs which I can reverse engineer?

        Thank you very much.

        No matter how hysterical I get, my problems are not time sensitive. So, relax, have a cookie, and a very nice day!
        Lady Aleena

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: note [id://1124075]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others browsing the Monastery: (3)
As of 2024-04-19 21:25 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found