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 mvc (Scribe)
on May 21, 2003 at 23:06 UTC ( [id://259928]=note: print w/replies, xml ) Need Help??


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

You are on the right road, but it seems as if nobody has mentioned your solution is not good OO design:

BAD:           if ($po->status->name eq 'Sent') ...
ALMOST AS BAD: if ($po->can_send) ...
GOOD:          $po->send

You are breaking encapsulation in all but the last case. Getters are a design smell. They are just a way to make private data public.

Instead of asking objects questions, tell them to do things. The object will figure out how to do it.

Allen Holub says about this:

"Never ask an object for the information you need to do 
something; rather, ask the object that has the information 
to do the work for you."
  • Comment on Re: Short Refactoring Tip: let the object do it for you

Replies are listed 'Best First'.
Re: Re: Short Refactoring Tip: let the object do it for you
by Ovid (Cardinal) on May 21, 2003 at 23:28 UTC

    That's a very interesting observation. Given what you've written, how would you handle this?

    [% IF item.can_receive %] <input type="text" name="received" value="[% item.received %]" cla +ss="input2"/> [% ELSE; item.received; END %]

    In the above case, I am querying whether or not an item can accept a value for given attribute (those are Template Toolkit directives). If so, an input box is presented to the end user, with the current value filled in. If not, they only see the value. As this data can be represented in many other ways (and not just on HTML forms), it doesn't seem to make sense to have the object present HTML code to the end user. I like the idea of moving the conditional logic into the object, but I don't see a clean way of doing that.

    Cheers,
    Ovid

    New address of my CGI Course.
    Silence is Evil (feel free to copy and distribute widely - note copyright text)

      Here we have a classic conflict of design forces:

      • Encapsulation
      • Seperation of concerns (cohesion)

      If you put the UI code in the object, you are encapsulated but your object is now doing its regular work + UI work (low cohesion). If you keep it as is, you get clean seperation of concerns, but break encapsulation.

      There are other solutions that could balance these forces better. The balance of forces you want depends on your requirements.

      You could, for example, have the template code look like this:

      <% item.getReceivedUI("<input ...>") %>
      

      The item object getReceivedUI() will check if receive is possible, if so interpolate the parameter as a TT template with item.received in the stash. If not, it returns item.received. Then you remove the UI code from the item object by delegating the actual interpolation to some strategy object, that may use TT or anything. Thus the item knows not about UI, just about presentation logic. The template is still in charge of the UI. Is this extra work worth it? Depends on your requirements.

Re: Re: Short Refactoring Tip: let the object do it for you
by dws (Chancellor) on May 22, 2003 at 03:33 UTC
    BAD: if ($po->status->name eq 'Sent') ... ALMOST AS BAD: if ($po->can_send) ... GOOD: $po->send
    You are breaking encapsulation in all but the last case.
    How, pray tell, does a predicate function "break encapsulation"? can_send is not a getter; it reveals nothing about how the purchase order answers the question. Encapsulation is quite safely preserved.

      Do you define "predicate function" here as "method that returns a boolean value"? If so, then it reveals something about the object state- information that should be encapsulated.

      What if you decide that "you can always send!". Then you have to remove the conditionals from all clients of your object. Or what if there are now 3 states for sending, each requiring a different send method? We want the clients to know as little as possible about the object. Knowing only one method, is better than knowing a method, its return type, and another method.

      "...The key question is: How much of one module must be 
      known in order to understand another module?..."
      

           Yourdon, Ed and Larry L. Constantine. Structured Design: Fundamentals of a Discipline of Computer Program and Systems Design. (1979) On Coupling, p.85

      Breaking encapsulation here increases coupling between the clients of the object and the object itself. Note I am not saying: never use boolean methods. There may be other design forces besides "encapsulate hermetically", so we do not know which solution is best here.

      Code like: "if an object can do this, tell it to do it" smells.

        Do you define "predicate function" here as "method that returns a boolean value"? If so, then it reveals something about the object state- information that should be encapsulated.

        A predicate method (or a query method, for that matter) defends encapsulation. The existence of the method says nothing about how the target object derives an answer. It could have the answer in a hash, it could calculate it lazily, it could delegate to another object. Whatever. The point is that the client doesn't know. Coupling is kept down. And, assuming that the question lies within the province of the target object to answer, cohesion is kept up.

        I see that what you're really arguing for is reducing the need for "can you do this? yes? then do it" types of protocols. Fine; I agree. But in arguing that point, you're either misunderstanding or misrepresenting encapsulation.

        Suppose you have code that says, "if object can't send email, send a fax". You end up writing an alerter-management object, then. I suppose you could keep going, but this would just create turtles all the way down until your program was nothing but objects and messages, just to avoid "scary" predicates before OO hype made this un-PC. If you want 1000 lines of code when 100 will do, be my guest.

Log In?
Username:
Password:

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

How do I use this?Last hourOther CB clients
Other Users?
Others sharing their wisdom with the Monastery: (5)
As of 2024-04-18 02:28 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found