Re: Tell, Don't Ask
- From: "H. S. Lahman" <h.lahman@xxxxxxxxxxx>
- Date: Sun, 14 May 2006 18:36:46 GMT
Responding to Daniel T....
This is mostly for the OP's benefit...
An example is in order. In most example texts, Stack::pop() has a precondition that Stack::empty() is false. We end up with code like this:
while not myStack.empty():
myStack.pop()
but the above is exactly what Hunt and Thomas are saying is bad practice, we are querying the state of the object in order to decide what to do with it. The precondition, in fact, *requires* us to make the query (or otherwise "know" that the stack in not empty.)
I think the answer lies in another recent thread about programming to interface. To me the real question here is whether Stack::pop should depend on Stack::empty. There may be valid reasons for a client to know whether the Stack is empty or not, but they just don't include needing to know in order to invoke Stack::pop.
Correcting the problem means that one has to move the decision about whether pop can be invoked out of the client and into Stack. Logically there is better cohesion if Stack::pop tests if an element can be returned rather than the client. IOW, how the Stack being empty affects popping elements is really a Stack concern.
But the client is expecting a element to be returned by Stack. I assert that client already needs to be prepared for the stack being empty. That is, when Stack::empty returns TRUE, then the client needs to do something other than processing an element. However, we can deal with that by having Stack::pop return NULL if the Stack is empty. Then client can test the /result/ for Stack::pop to determine what to do.
This is a better solution because the client doesn't care why a NULL value is returned. All it needs to understand is that popping does not always produce an element, which it needed to understand anyway.
Another example:
class Range:
"invariant: self.getLow() < self.getHigh()"
def getLow(self):
return self.low
def getHigh(self):
return self.high
def setHigh(self, value):
"require: value > self.getLow()"
self.high = value
def setLow(self, value):
"require: value < self.getHigh()"
self.low = value
Sure the invariant will be kept, but not because of any effort on the part of the Range class, users of Range are the ones that have to make sure the invariant holds... But it's not supposed to be their job, that's the job of Range.
Indeed. But easily fixed if one moves the business rules and policies about the relative values of Low and High into Range. For example (ignoring quibbles about equality):
def setHigh(self, value)
if (value > self.low)
self.high = value
Assuming the invariant is all that we are worried about, we really don't need the precondition on value. In that case, even better would be:
def setValue(self, value)
if (value > self.low)
self.high = value
else
self.high = self.low
self.low = value
This is a bit more flexible than your last solution. However, this approach fails when something else in the overall solution context says that the client really never should supply a value less than the current self.low. That is, the precondition is really a correctness check on business rules and policies implemented elsewhere.
For example, the low and high values might need to be consistent as a pair but they are provided from different contexts whose order of invocation is undefined. Then one can't use the second example for sure and may not be able to use the first because 'value' may be out of synch in time (i.e., two high values before a low). I submit that in those sorts of situations the preconditions and your initial solution are reasonable because it really is someone else's job to ensure synchronization of updating the pair of values.
Having said that, I think there are trade-offs that can still be made about the responsibilities of external clients, at least in some situations. Let's assume my example where values must be set in pairs but an arbitrary order prevails. We can add two flags, newHighProvided and newLowProvided, along with two temporary value stores, newHighValue and newLowValue, to Range. Then we might have:
def reset(self)
self.newHighProvided = FALSE
self.newLowProvided = FALSE
def setHigh(self, value)
"require self.newHighProvided = FALSE"
"require self.newLowProvided = FALSE ||
(self.newLowProvided = TRUE &&
value > self.newLowProvided)
self.newHighValue = value
self.newHighProvided = TRUE
if (newLowProvided)
self.setValues()
def setLow(self, value)
// symmetric with setHigh
def setValues
"invariant self.newLowValue < self.newHighValue"
self.low = self.newLowValue
self.High = self. newHighValue
This changes teh client contracts in subtle ways. Now all we need for the context to understand is that Range::reset must be invoked before new values are determined and that values must be supplied in pairs. The synchronization of producing consistent pairs is enforced by the precondition, not the relative value. Indirectly the context still needs to know that the high value should be greater than the low value. But that is implicit in Range::setHigh and Range::setLow already. The invariant is really a correctness check that /pairs/ are produced correctly. Overall I think this is probably a better way to capture the constraint nuances of the external processing than checking relative values _given the stipulated problem_.
The downside, of course, is that this is more elaborate and it is custom tailored to a very particular problem context. I think whether one wants to (or can, if there are other nonfunctional requirements) go to this trouble to make the preconditions more generic is a trade-off to be made in the the design of collaborations. But, given the stipulated problem, somebody besides Range is going to have to understand something about the rules and policies of collecting pairs of values.
[One can also argue that I am solving a different problem. B-) My counter is that, in practice, I would bet that setting any Range involves similar issues whenever the situation warrants different interface methods for high and low values. Otherwise, one would have a single interface of Range::setLimits (self, lowVlaue, highValue).]
*************
There is nothing wrong with me that could
not be cured by a capful of Drano.
H. S. Lahman
hsl@xxxxxxxxxxxxxxxxx
Pathfinder Solutions -- Put MDA to Work
http://www.pathfindermda.com
blog: http://pathfinderpeople.blogs.com/hslahman
Pathfinder is hiring: http://www.pathfindermda.com/about_us/careers_pos3.php.
(888)OOA-PATH
.
- Follow-Ups:
- Re: Tell, Don't Ask
- From: Daniel T.
- Re: Tell, Don't Ask
- References:
- Tell, Don't Ask
- From: Squeamizh
- Re: Tell, Don't Ask
- From: Daniel T.
- Tell, Don't Ask
- Prev by Date: Re: Searching OO Associations with RDBMS Persistence Models
- Next by Date: Re: Programming to an Interface
- Previous by thread: Re: Tell, Don't Ask
- Next by thread: Re: Tell, Don't Ask
- Index(es):
Relevant Pages
|
|