About the Announcements code

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

About the Announcements code

Manfred Kröhnert
Hi,

while creating the pull request to add Announcer>>on:doOnce: I stumbled over two things.

First, the SystemAnnouncement class has a member named 'theClass' as well as accessor methods.
However, both ClassAnnouncement and ProtocolAnnouncement contain the same member + accessors.
In my opinion the member should be removed from SystemAnnouncement since it does not generalize for all Announcement subclasses.
Any other opinions?

Second, I tried to write a unit test for Announcer>>unsubscribe: which proved to be more difficult than I thought.
The >>unsubscribe: method takes an object and removes it from the list of subscriptions.
However, the objects stored therein are AnnouncementSubscriptions which are created in >>on:do:.
However, >>unsubscribe: uses its argument to remove it from the subscription list.
Which is not easy since you would have to create another AnnouncementSubscription object.

Then I had a look at Pharo and they store the receiver of the announcement as well.
It turns out that BlockClosure>>receiver returns nil in Amber in all cases rendering this option useless as of now.

So the question is how we can fix this issue.

Best,
Manfred

--
You received this message because you are subscribed to the Google Groups "amber-lang" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/groups/opt_out.
Reply | Threaded
Open this post in threaded view
|

Re: About the Announcements code

Nicolas Petton

Manfred Kröhnert writes:

> Hi,
>
> while creating the pull request to add Announcer>>on:doOnce: I stumbled
> over two things.
>
> First, the SystemAnnouncement class has a member named 'theClass' as well
> as accessor methods.
> However, both ClassAnnouncement and ProtocolAnnouncement contain the same
> member + accessors.
> In my opinion the member should be removed from SystemAnnouncement since it
> does not generalize for all Announcement subclasses.
> Any other opinions?

Yes, you're right :)

Nico

--
You received this message because you are subscribed to the Google Groups "amber-lang" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/groups/opt_out.
Reply | Threaded
Open this post in threaded view
|

Re: About the Announcements code

Manfred Kröhnert
Hi Nico,


On Tue, Nov 26, 2013 at 5:38 PM, Nicolas Petton <[hidden email]> wrote:

Manfred Kröhnert writes:

> Hi,
>
> while creating the pull request to add Announcer>>on:doOnce: I stumbled
> over two things.
>
> First, the SystemAnnouncement class has a member named 'theClass' as well
> as accessor methods.
> However, both ClassAnnouncement and ProtocolAnnouncement contain the same
> member + accessors.
> In my opinion the member should be removed from SystemAnnouncement since it
> does not generalize for all Announcement subclasses.
> Any other opinions?

Yes, you're right :)

Nico


What about the other question?

Best,
Manfred 

--
You received this message because you are subscribed to the Google Groups "amber-lang" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/groups/opt_out.