[Pharo-project] Event smell...
Hi igor I started to reread again the code of EventModel that we started and I browse the rest of the systemâ¦. gloops⦠DropEvent>>sentTo: anObject "Dispatch the receiver into anObject" self type == #dropEvent ifTrue:[^anObject handleDropMorph: self]. DropFilesEvent>>sentTo: anObject "Dispatch the receiver into anObject" self type == #dropFilesEvent ifTrue:[^anObject handleDropFiles: self]. it seems to me that sentTo: is a kind of double dispatch but not yet there. I do not see here the added value of the check except if this is to cancel event. Stef
On 11.09.2012 14:35, Stéphane Ducasse wrote:
Hi igor
I started to reread again the code of EventModel that we started and I browse the rest of the systemâ¦. gloopsâ¦
DropEvent>>sentTo: anObject "Dispatch the receiver into anObject" self type == #dropEvent ifTrue:[^anObject handleDropMorph: self].
DropFilesEvent>>sentTo: anObject "Dispatch the receiver into anObject" self type == #dropFilesEvent ifTrue:[^anObject handleDropFiles: self].
it seems to me that sentTo: is a kind of double dispatch but not yet there. I do not see here the added value of the check except if this is to cancel event.
Stef I guess it's to be consistent with other types of events, and/or make it easy to add a type instvar in the future... Right now all three combinations of type instvar checks (KeyboardEvent), subclassing (DropXEvents), or a mix of the two (MouseEvents) are in use during the double dispatching of input events. You might think subclassing was only used if additional state was needed, but that's only true of MouseEvents, and not DropXEvents... (So yeah, your observation that the type check in DropEvent is unneccesary is correct)
It might be nice with a rewrite to rely on subclassing alone, albeit increaseing the number of event classes. (KeyUp, KeyDown, KeyPressed, MouseOver, MouseEnter and MouseLeave for sure, potentially more depending on what action in WindowEvent means) A benefit is you could rewrite processEvents loop to essentially read event := MorphicEvent forBuffer: evtBuf. self handleEvent: event (What is recorded in recentModifiers should be queried directly from event during handling, not through HandMorph) And it would also make rewriting handleEvent: to something less... intrusive wrt Event modelling a much simpler task later on. Cheers, Henry
On 11 September 2012 15:27, Henrik Sperre Johansen <henrik.s.johansen@veloxit.no> wrote:
On 11.09.2012 14:35, Stéphane Ducasse wrote:
Hi igor
I started to reread again the code of EventModel that we started and I browse the rest of the systemâ¦. gloopsâ¦
DropEvent>>sentTo: anObject "Dispatch the receiver into anObject" self type == #dropEvent ifTrue:[^anObject handleDropMorph: self].
DropFilesEvent>>sentTo: anObject "Dispatch the receiver into anObject" self type == #dropFilesEvent ifTrue:[^anObject handleDropFiles: self].
it seems to me that sentTo: is a kind of double dispatch but not yet there. I do not see here the added value of the check except if this is to cancel event.
Stef
I guess it's to be consistent with other types of events, and/or make it easy to add a type instvar in the future... Right now all three combinations of type instvar checks (KeyboardEvent), subclassing (DropXEvents), or a mix of the two (MouseEvents) are in use during the double dispatching of input events. You might think subclassing was only used if additional state was needed, but that's only true of MouseEvents, and not DropXEvents... (So yeah, your observation that the type check in DropEvent is unneccesary is correct)
It might be nice with a rewrite to rely on subclassing alone, albeit increaseing the number of event classes. (KeyUp, KeyDown, KeyPressed, MouseOver, MouseEnter and MouseLeave for sure, potentially more depending on what action in WindowEvent means)
A benefit is you could rewrite processEvents loop to essentially read event := MorphicEvent forBuffer: evtBuf. self handleEvent: event
yes, that's the idea. but we didn't went further than passing events to HandMorph. At first stage we want to make sure a lower-level event handling (Sensor->World->HandMorph) can be replaced with new model, and then we can enter the morphic and clean it up more seriously. Since doing everything at once, will not be a good idea, because of the amount of work and number of changes you might need to do to system.
(What is recorded in recentModifiers should be queried directly from event during handling, not through HandMorph) And it would also make rewriting handleEvent: to something less... intrusive wrt Event modelling a much simpler task later on.
yes yes yes :)
Cheers, Henry
-- Best regards, Igor Stasenko.
On 11.09.2012 15:41, Igor Stasenko wrote:
On 11 September 2012 15:27, Henrik Sperre Johansen <henrik.s.johansen@veloxit.no> wrote:
On 11.09.2012 14:35, Stéphane Ducasse wrote:
Hi igor
I started to reread again the code of EventModel that we started and I browse the rest of the systemâ¦. gloopsâ¦
DropEvent>>sentTo: anObject "Dispatch the receiver into anObject" self type == #dropEvent ifTrue:[^anObject handleDropMorph: self].
DropFilesEvent>>sentTo: anObject "Dispatch the receiver into anObject" self type == #dropFilesEvent ifTrue:[^anObject handleDropFiles: self].
it seems to me that sentTo: is a kind of double dispatch but not yet there. I do not see here the added value of the check except if this is to cancel event.
Stef I guess it's to be consistent with other types of events, and/or make it easy to add a type instvar in the future... Right now all three combinations of type instvar checks (KeyboardEvent), subclassing (DropXEvents), or a mix of the two (MouseEvents) are in use during the double dispatching of input events. You might think subclassing was only used if additional state was needed, but that's only true of MouseEvents, and not DropXEvents... (So yeah, your observation that the type check in DropEvent is unneccesary is correct)
It might be nice with a rewrite to rely on subclassing alone, albeit increaseing the number of event classes. (KeyUp, KeyDown, KeyPressed, MouseOver, MouseEnter and MouseLeave for sure, potentially more depending on what action in WindowEvent means)
A benefit is you could rewrite processEvents loop to essentially read event := MorphicEvent forBuffer: evtBuf. self handleEvent: event
yes, that's the idea. but we didn't went further than passing events to HandMorph. At first stage we want to make sure a lower-level event handling (Sensor->World->HandMorph) can be replaced with new model, and then we can enter the morphic and clean it up more seriously. Mmmmhmm, isn't it more accurate to say that events flow Sensor -> HandMorph -> World? (Though, they get pulled in by the consuming world in a polling loop, rather than getting pushed from the flow source)
Anyways, a proper input event modelling is really independent of Morphic, analogous to the WindowClosed/MorphicWindowClosed naming discussion. (which also gives a third alternative to double dispatch/case, in that in use registering to them tend to look like case statements, but loosely coupled) Cheers, Henry PS: Looking up an example WindowAnnouncement and ran across it; WTF does Nautilus need special NautilusWindowAnnouncement classes for? As long as your announcer is a Nautilus-specific one rather than the default announcer, that's all the separation that's needed... In other words; favour application-distinct announcers over duplicate Announcement types.
Henrik have a look at EventModel/EventModel because this is were we publish our vm event brain surgery.
yes, that's the idea. but we didn't went further than passing events to HandMorph. At first stage we want to make sure a lower-level event handling (Sensor->World->HandMorph) can be replaced with new model, and then we can enter the morphic and clean it up more seriously. Mmmmhmm, isn't it more accurate to say that events flow Sensor -> HandMorph -> World? (Though, they get pulled in by the consuming world in a polling loop, rather than getting pushed from the flow source)
Anyways, a proper input event modelling is really independent of Morphic, analogous to the WindowClosed/MorphicWindowClosed naming discussion. (which also gives a third alternative to double dispatch/case, in that in use registering to them tend to look like case statements, but loosely coupled)
Cheers, Henry
PS: Looking up an example WindowAnnouncement and ran across it; WTF does Nautilus need special NautilusWindowAnnouncement classes for? As long as your announcer is a Nautilus-specific one rather than the default announcer, that's all the separation that's needed... In other words; favour application-distinct announcers over duplicate Announcement types.
Open a bug entry and send code. I'm sure ben will be happy
On Sep 11, 2012, at 4:23 PM, Henrik Sperre Johansen wrote:
On 11.09.2012 15:41, Igor Stasenko wrote:
On 11 September 2012 15:27, Henrik Sperre Johansen <henrik.s.johansen@veloxit.no> wrote:
On 11.09.2012 14:35, Stéphane Ducasse wrote:
Hi igor
I started to reread again the code of EventModel that we started and I browse the rest of the systemâ¦. gloopsâ¦
DropEvent>>sentTo: anObject "Dispatch the receiver into anObject" self type == #dropEvent ifTrue:[^anObject handleDropMorph: self].
DropFilesEvent>>sentTo: anObject "Dispatch the receiver into anObject" self type == #dropFilesEvent ifTrue:[^anObject handleDropFiles: self].
it seems to me that sentTo: is a kind of double dispatch but not yet there. I do not see here the added value of the check except if this is to cancel event.
Stef I guess it's to be consistent with other types of events, and/or make it easy to add a type instvar in the future... Right now all three combinations of type instvar checks (KeyboardEvent), subclassing (DropXEvents), or a mix of the two (MouseEvents) are in use during the double dispatching of input events. You might think subclassing was only used if additional state was needed, but that's only true of MouseEvents, and not DropXEvents... (So yeah, your observation that the type check in DropEvent is unneccesary is correct)
It might be nice with a rewrite to rely on subclassing alone, albeit increaseing the number of event classes. (KeyUp, KeyDown, KeyPressed, MouseOver, MouseEnter and MouseLeave for sure, potentially more depending on what action in WindowEvent means)
A benefit is you could rewrite processEvents loop to essentially read event := MorphicEvent forBuffer: evtBuf. self handleEvent: event
yes, that's the idea. but we didn't went further than passing events to HandMorph. At first stage we want to make sure a lower-level event handling (Sensor->World->HandMorph) can be replaced with new model, and then we can enter the morphic and clean it up more seriously. Mmmmhmm, isn't it more accurate to say that events flow Sensor -> HandMorph -> World? (Though, they get pulled in by the consuming world in a polling loop, rather than getting pushed from the flow source)
Anyways, a proper input event modelling is really independent of Morphic, analogous to the WindowClosed/MorphicWindowClosed naming discussion. (which also gives a third alternative to double dispatch/case, in that in use registering to them tend to look like case statements, but loosely coupled)
Cheers, Henry
PS: Looking up an example WindowAnnouncement and ran across it; WTF does Nautilus need special NautilusWindowAnnouncement classes for? As long as your announcer is a Nautilus-specific one rather than the default announcer, that's all the separation that's needed... In other words; favour application-distinct announcers over duplicate Announcement types.
I think it's just because I wasn't aware that this class existed :) I should fix that Ben
On 11.09.2012 18:27, Benjamin wrote:
PS: Looking up an example WindowAnnouncement and ran across it; WTF does Nautilus need special NautilusWindowAnnouncement classes for? As long as your announcer is a Nautilus-specific one rather than the default announcer, that's all the separation that's needed... In other words; favour application-distinct announcers over duplicate Announcement types. I think it's just because I wasn't aware that this class existed :)
I should fix that
Ben
Great! I made you an issue so you can't forget :) http://code.google.com/p/pharo/issues/detail?id=6659 Cheers, Henry
Thanks :) Ben On Sep 12, 2012, at 9:53 AM, Henrik Sperre Johansen wrote:
On 11.09.2012 18:27, Benjamin wrote:
PS: Looking up an example WindowAnnouncement and ran across it; WTF does Nautilus need special NautilusWindowAnnouncement classes for? As long as your announcer is a Nautilus-specific one rather than the default announcer, that's all the separation that's needed... In other words; favour application-distinct announcers over duplicate Announcement types. I think it's just because I wasn't aware that this class existed :)
I should fix that
Ben
Great! I made you an issue so you can't forget :)
http://code.google.com/p/pharo/issues/detail?id=6659
Cheers, Henry
I guess it's to be consistent with other types of events, and/or make it easy to add a type instvar in the future... Right now all three combinations of type instvar checks (KeyboardEvent), subclassing (DropXEvents), or a mix of the two (MouseEvents) are in use during the double dispatching of input events. You might think subclassing was only used if additional state was needed, but that's only true of MouseEvents, and not DropXEvents... (So yeah, your observation that the type check in DropEvent is unneccesary is correct)
It might be nice with a rewrite to rely on subclassing alone, albeit increaseing the number of event classes. (KeyUp, KeyDown, KeyPressed, MouseOver, MouseEnter and MouseLeave for sure, potentially more depending on what action in WindowEvent means)
A benefit is you could rewrite processEvents loop to essentially read event := MorphicEvent forBuffer: evtBuf. self handleEvent: event
yes this is something like that that I would like.
(What is recorded in recentModifiers should be queried directly from event during handling, not through HandMorph) And it would also make rewriting handleEvent: to something less... intrusive wrt Event modelling a much simpler task later on.
Yes I'm taking notes about these points. Now the first level for me is to handle raw VM events then after we will deal with morphic ones.
Cheers, Henry
On 11 September 2012 14:35, Stéphane Ducasse <Stephane.Ducasse@inria.fr> wrote:
Hi igor
I started to reread again the code of EventModel that we started and I browse the rest of the systemâ¦. gloopsâ¦
DropEvent>>sentTo: anObject "Dispatch the receiver into anObject" self type == #dropEvent ifTrue:[^anObject handleDropMorph: self].
DropFilesEvent>>sentTo: anObject "Dispatch the receiver into anObject" self type == #dropFilesEvent ifTrue:[^anObject handleDropFiles: self].
it seems to me that sentTo: is a kind of double dispatch but not yet there. I do not see here the added value of the check except if this is to cancel event.
rofl.. you managed to force me to say "wtf" loudly, just after i seen these two methods. Congratulations! :) And after looking at those classes, i found that those checks is completely useless because they are always yielding true: DropEvent>>type ^#dropEvent DropFilesEvent>>type ^#dropFilesEvent and dispatch is always performed. And i don't see why would anyone want to cancel event in such fashion, because if you don't wanna handle it, then just don't send #sentTo: or make sure that end receiver of double-dispatch just ignores such kind of events. I must admit, that a double-dispatch is one of my favorite hammers i use , because i think it one of the best ways for dealing with polymorphism. But before i started using smalltalk i almost never used double-dispatch in my code, and in other places where i found it , was confusing me a lot.. This is because in other languages you tend to use case statements instead, because the amount of coding to implement double-dispatch is too verbose comparing to little case-statement .. this is of course says a lot about expressive power of smalltalk.. and of course, it would be much more difficult to determine if given piece of code actually a double-dispatch or not, unless you can see all implementors & senders using browser, but instead will have to go through multiple files to determine that (because again, most so-called OO languages best practices saying that you should use one file per class). -- Best regards, Igor Stasenko.
participants (5)
-
Benjamin -
Henrik Sperre Johansen -
Igor Stasenko -
Stéphane Ducasse -
Stéphane Ducasse