OOP best practices
Locked
 
|
Hi Hernan, just wanted to get your opinion on the following method, that attempts to adhere to your "complete objects" pattern. ( ESUG 2010 TALK).
PerformedExperiment>>experiment: anExperiment start: aTime stop: aStopTime participant: aParticipant tasks: aCollectionOfTasks | experiment | experiment := self new. experiment experiment: anExperiment start: aTime stop: aStopTime participant: aParticipant tasks: aCollectionOfTasks. ^ experiment In your experience, how does the pattern cope with large keyword selectors. Maybe this case is not that evident, although we should try to avoid having more than 4 instance variables anyway, i would like to get your opinion on this problem (?). Thanks, Fernando _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
|
|
Hi Fernando,
I think that you are saying that having too many parameter could be a problem for understanding the message, is that right? I mean, having too many inst var as you say, it is a smell of bad design, and having too many parameters also, so I guess we agree on that.
About reading the code, from the sender point of view, if you don't have an instance creation message that creates a "complete" object, then you would do something like this: performedExperiment := PerformedExperiment new
experimient: anExperiment; start: aStartTime ; stop: aStopTime ; participant: aParticipant; tasks: aCollectionOfTasks; yourself. With an inst. creation message that returns a complete object, you would use it like this: performedExperiment := PerformedExperiment experimient: anExperiment start: aStartTime
stop: aStopTime participant: aParticipant tasks: aCollectionOfTasks. So, it is basically the same but with less messages and less error prone (in the former you can forget to send a message, ie. stop: and nobody will complain immediately, but in the last one you wont make that mistake, or if you doit you will get a dnu immediately).
About the implementation of the inst. creation message, it is true that it could bother a little the reading, but when there are so many parameters I format the code this way:
PerformedExperiment class>>experimient: anExperiment start: aStartTime stop: aStopTime participant: aParticipant tasks: aCollectionOfTasks ^self new initializeExperimient: anExperiment start: aStartTime stop: aStopTime participant: aParticipant
tasks: aCollectionOfTasks that makes it more readable. Also look that there is an initializeXxx message to distinguish it from the inst. creation message. This helps when analyzing code automatically (ie. initialize messages should only be sent from the class side) or generating code automatically on the debugger with the create button (as the enhancement 3099 that I sent the other day, the debugger could be smart enough to realize it is an initialization message so it could provide a better template than just a "self shouldBeImplemented").
I the meantime, I would suggest another name for the message you are using as example, something easier to read like: performedExperiment := PerformedExperiment for: anExperiment
startedAt: aStartTime stopedAt: aStopTime madeBy: aParticipant "Not sure what participant is, so maybe madeBy is not a good name" with: aCollectionOfTasks.
I think it reads better. With the former you have duplicated "names" like "experimient: anExperiment", "start: anStartTime", etc., with the last one you don't have that problem.
Hope it helps, let me know what you think! Hernan. On Thu, Oct 14, 2010 at 4:48 PM, Fernando olivero <[hidden email]> wrote: Hi Hernan, just wanted to get your opinion on the following method, that attempts to adhere to your "complete objects" pattern. ( ESUG 2010 TALK). -- Hernán Wilkinson
Agile Software Development, Teaching & Coaching Mobile: +54 - 911 - 4470 - 7207 email: [hidden email] site: http://www.10Pines.com _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
|
|
In reply to this post by Fernando olivero-2
> PerformedExperiment>>experiment: anExperiment start: aTime stop: aStopTime participant: aParticipant tasks: aCollectionOfTasks
I guess you meant PerformedExperiment class >>experiment: ... Alexandre -- _,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;: Alexandre Bergel http://www.bergel.eu ^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;. _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
|
|
In reply to this post by Hernan Wilkinson-3
THANKS Hernan!
With the example i've got to understand the correct usage of the pattern now. Which of course i will follow from now on, given the benefits from having complete objects! ( no messy ifNil checks for starters!, etc...) On Oct 14, 2010, at 10:35 PM, Hernan Wilkinson wrote: Hi Fernando, _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
|
|
In reply to this post by Hernan Wilkinson-3
So a "smart template" created by the debugger would just do the following, when defining the following Class method: performedExperiment := PerformedExperiment for: anExperiment startedAt: aStartTime stopedAt: aStopTime madeBy: aParticipant "Not sure what participant is, so maybe madeBy is not a good name" with: aCollectionOfTasks. DNU => HIT CREATE BUTTON => Automatically create the following methods 1) PerformedExperiment class >> for: anExperiment startedAt: aStartTime stopedAt: aStopTime madeBy: aParticipant with: aCollectionOfTasks. | instance | instance := self new. instance initializeFor: anExperiment startedAt: aStartTime stopedAt: aStopTime madeBy: aParticipant with: aCollectionOfTasks. ^ instance 2) PerformedExperiment>> initializeFor: anExperiment startedAt: aStartTime stopedAt: aStopTime madeBy: aParticipant with: aCollectionOfTasks. self shouldBeImplemented On Oct 14, 2010, at 10:35 PM, Hernan Wilkinson wrote: Hi Fernando, _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
|
|
In reply to this post by Fernando olivero-2
cool!
remember also the other tip, that the object should be valid from scratch... so, for this example I would do something like this: startedAt: aStartTime stopedAt: aStopTime madeBy: aParticipant with: aCollectionOfTasks PatAssertionsRunner valueWith: (self assertionForPerformedExperimentStartedAt: aStartTime before: aStopTime). ^self new initializeExperimient: anExperiment start: aStartTime stop: aStopTime participant: aParticipant tasks: aCollectionOfTasks That way if the stoptime is before starttime (wich I beleive is an error in this case) the object is not created and you wont have an invalid performed experiment.
You will see that asserting that something starts before the "stop" is very common, so I suggest to use an interval to represent that but not the smalltalk Interval that does not verify that condition but the Aconcagua Interval that verifies that intervals are valid. So you could change the message to something like this:
PerformedExperiment class>>for: anExperiment during: aTimePeriod madeBy: aParticipant with: aCollectionOfTasks ^self new initializeExperimient: anExperiment
during: aTimePeriod participant: aParticipant tasks: aCollectionOfTasks Doing so you get less parameters and only valid "time periods" (intervals) that leads to valid performed experiments.
Another assertion that you may have is if aCollectionOfTasks can be empty... anyway, thinking about this constrains helps you to create a better model and understand the business rules. I hope it helps!
Hernan. 2010/10/14 Fernando olivero <[hidden email]>
--
Hernán Wilkinson
Agile Software Development, Teaching & Coaching Mobile: +54 - 911 - 4470 - 7207 email: [hidden email] site: http://www.10Pines.com _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
|
|
In reply to this post by Fernando olivero-2
Almost... :-) it would be:
performedExperiment := PerformedExperiment for: anExperiment startedAt: aStartTime stopedAt: aStopTime
madeBy: aParticipant "Not sure what participant is, so maybe madeBy is not a good name" with: aCollectionOfTasks. DNU => HIT CREATE BUTTON => Automatically create the following methods
1) PerformedExperiment class >> for: anExperiment startedAt: aStartTime stopedAt: aStopTime madeBy: aParticipant
with: aCollectionOfTasks. | instance | instance := self new. instance
initializeFor: anExperiment startedAt: aStartTime stopedAt: aStopTime
madeBy: aParticipant with: aCollectionOfTasks.
^ instance Save - Proceed - DNU -> Create Button ->
2) PerformedExperiment>> initializeFor: anExperiment startedAt: aStartTime stopedAt: aStopTime madeBy: aParticipant with: aCollectionOfTasks.
self shouldBeImplemented. " Suggested code - Uncomment it if you like it :-)
experiment := anExperiment. startTime := aStartTime. stopTime := aStopTime. participant := aParticipant. taks := aCollectionOfTasks. " The template can be created that way because it knows it is an initialize message, so it takes the parameter names to suggest the inst var names and assign to them the right parameters. Having these kind of "idioms" can really help when creating code automatically in such a context like the debugger
2010/10/14 Fernando olivero <[hidden email]>
--
Hernán Wilkinson
Agile Software Development, Teaching & Coaching Mobile: +54 - 911 - 4470 - 7207 email: [hidden email] site: http://www.10Pines.com _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
|
|
I don't really like this way of formatting the message with line breaks. I agree on Inline Message Pattern in Kent Beck's Smalltalk Best Practice Patterns. There's the draft here
Look at p.127 / 128 Cheers, Laurent _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
|
|
I don't like it either, but sometimes you have to use it...
2010/10/15 laurent laffont <[hidden email]>
--
Hernán Wilkinson
Agile Software Development, Teaching & Coaching Mobile: +54 - 911 - 4470 - 7207 email: [hidden email] site: http://www.10Pines.com _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
«
Return to Pharo Smalltalk Developers
|
1 view|%1 views
| Free forum by Nabble | Edit this page |