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

Re: Short Refactoring Tip: let the object do it for you

by demerphq (Chancellor)
on May 19, 2003 at 22:49 UTC ( #259332=note: print w/ replies, xml ) Need Help??


in reply to Short Refactoring Tip: let the object do it for you

Id like to point out that this doesn't scale very well. Sure you have a valid point in a limited case, such as when there are only a few relevent states. But what happens when there are lots? Or the system is dynamic? Or if you want to say things like

if ($po->status->name =~ /^(sent|pending|waiting|procratinating|delaye +d)$/i) {

I'd rather write the above than call a bunch of methods, let alone write the methods in the first place. :-)

I thinks its unlikely I would write an object that require on a regular basis the double indirection like that. And if I did face a situation like my example above I'd probably do something more like

my $status=$pos->status; if ($status->is('Pending')) {} if ($status->in(qw(sent pending waitning procrastinating delayed)) { }

The issue here for me is that the use of 'Pending' isn't wrong, its the use of 'Pending' in such a way that I can't intercept that usage that is wrong. The subs above allow use to redefine the back end behaviour just as yours does, but its a lot more flexible an approach, while still providing us a way to block inapropriate checks. For instance $status->is('Pneding') should produce an error. Albeit a run time one. Oh well you cant have everything can you? :-)

Anyway, I use both styles as seems appropriate for the task at hand, often both at the same time even. :-)


---
demerphq

<Elian> And I do take a kind of perverse pleasure in having an OO assembly language...


Comment on Re: Short Refactoring Tip: let the object do it for you
Select or Download Code
Re: Re: Short Refactoring Tip: let the object do it for you
by hardburn (Abbot) on May 20, 2003 at 03:11 UTC

    I'd probably work that out as a dispatch table instead:

    my %STATES = ( sent => sub { . . . }, pending => sub { . . . }, waiting => sub { . . . }, procrastinating => sub { . . . }, delayed => sub { . . . }, ); # Make 'pending' and 'waiting' the same thing $STATES{pending} = $STATES{waiting}; # Later $STATES{$status->state->name}->();

    It's faster than a speeding regex. And I personally think dispatch tables are sexy.

    ----
    I wanted to explore how Perl's closures can be manipulated, and ended up creating an object system by accident.
    -- Schemer

    Note: All code is untested, unless otherwise stated

Re: Re: Short Refactoring Tip: let the object do it for you
by dws (Chancellor) on May 20, 2003 at 03:35 UTC
    I'd like to point out that this doesn't scale very well. ...
    if ($po->status->name =~ /^(sent|pending|waiting|procratinating|delaye +d)$/i) {
    I'd rather write the above than call a bunch of methods, let alone write the methods in the first place. :-)
    But you (well, I) wouldn't write individual accessor methods for all of those status names. Instead, write a predicate (a method that returns true or false) that describes what that combination of states means in terms of the behavior that a PurchaseOrder object exports, and in terms of the protocol that clients of PurchaseOrder expect to speak to it with. It's a lot easier to write unit tests that way, since you don't have to worry about arbitrary combinations of status bits. The valid combinations are hidden behind testable predicates. In fact, starting from predicates and working backwards to status values is one way to ensure that you don't have status values that you don't really need.

Re: Re: Short Refactoring Tip: let the object do it for you
by crenz (Priest) on May 20, 2003 at 14:06 UTC

    if ($po->status->name =~ /^(sent|pending|waiting|procratinating|delayed)$/i) {

    Shouldn't that be "procrastinating"? That typo already shows the dangers that lurk in this approach :).

Log In?
Username:
Password:

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

How do I use this? | Other CB clients
Other Users?
Others meditating upon the Monastery: (13)
As of 2014-08-27 21:05 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    The best computer themed movie is:











    Results (253 votes), past polls