You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@reef.apache.org by Dongjoon Hyun <do...@apache.org> on 2015/09/28 08:28:51 UTC
EventHandler class naming consistency policy?
Hi, all.
Today, I found 42 ActiveContextHandler usages and 42 ContextActiveHandler
usage in REEF. Of course, I know this is just a developer's choice in
general. But, as an Apache Top-level project project, I hope REEF shows
more consistent codes for new developers.
For example, in the following REEF document,
http://reef.apache.org/apidocs/0.12.0-incubating/org/apache/reef/wake/EventHandler.html
1. ActiveContextHandler classes
- JobDriver.ActiveContextHandler
- FailDriverDelayedMsg.ActiveContextHandler
- JobDriver.DriverRestartActiveContextHandler
- OutputServiceDriver.ActiveContextHandler
2. ContextActiveHandler classes
- DefaultContextActiveHandler
- BroadcastDriver.ContextActiveHandler
- DefaultDriverRestartContextActiveHandler
- LineCounter.ContextActiveHandler
The same situations observed in the other classes like TaskCompletedHandler
and CompletedTaskHandler, too.
I'm not quite sure if there is any reason for that. But, if there is no
reason, what about making a policy in REEF for naming of EventHandler class
before upcoming release? I think it's an easy fix.
Regards,
Dongjoon.
Re: EventHandler class naming consistency policy?
Posted by sh...@gmail.com.
I agree with that proposal.
I would like to also mention that I have come across GroupCommDriver and CommunicationGroupDriver.
> On Sep 28, 2015, at 1:41 PM, Markus Weimer <ma...@weimo.de> wrote:
>
>> On 2015-09-28 09:04, Dongjoon Hyun wrote:
>> I see. Then, at least, I can expect 'ContextActiveHandler',
>> 'TaskRunningHandler' and 'TaskFailedHandler' will be dominant form in
>> the future.
>
> Yes. We could make it a coding guideline as well.
>
> What do others think?
>
> Markus
Re: EventHandler class naming consistency policy?
Posted by Markus Weimer <ma...@weimo.de>.
On 2015-09-28 09:04, Dongjoon Hyun wrote:
> I see. Then, at least, I can expect 'ContextActiveHandler',
> 'TaskRunningHandler' and 'TaskFailedHandler' will be dominant form in
> the future.
Yes. We could make it a coding guideline as well.
What do others think?
Markus
Re: EventHandler class naming consistency policy?
Posted by Dongjoon Hyun <do...@apache.org>.
Thank you for your kind explanation and clear recommendation.
I see. Then, at least, I can expect 'ContextActiveHandler',
'TaskRunningHandler' and 'TaskFailedHandler' will be dominant form in the
future.
On Tue, Sep 29, 2015 at 12:15 AM, Markus Weimer <ma...@weimo.de> wrote:
> Hi,
>
> as with most things naming, the situation can be explained
> historically. Initially `RunningTaskHandler` sounded more appealing.
> But over time, we realized that `TaskRunningHandler` makes it much
> easier to control-space into the names. If it wouldn't break so much
> code, I'd even be in favor of renaming events that way, e.g.
> `RunningTask` would become `TaskRunningEvent` or such for the same
> reason.
>
> That said, the confusing cases we have in the code right now seem to
> be in examples and tests only. That should indeed be easy to fix, as
> we don't promise compatibility between releases for that.
>
> Markus
>
> On Sun, Sep 27, 2015 at 11:28 PM, Dongjoon Hyun <do...@apache.org>
> wrote:
> > Hi, all.
> >
> > Today, I found 42 ActiveContextHandler usages and 42 ContextActiveHandler
> > usage in REEF. Of course, I know this is just a developer's choice in
> > general. But, as an Apache Top-level project project, I hope REEF shows
> > more consistent codes for new developers.
> >
> > For example, in the following REEF document,
> >
> http://reef.apache.org/apidocs/0.12.0-incubating/org/apache/reef/wake/EventHandler.html
> >
> > 1. ActiveContextHandler classes
> > - JobDriver.ActiveContextHandler
> > - FailDriverDelayedMsg.ActiveContextHandler
> > - JobDriver.DriverRestartActiveContextHandler
> > - OutputServiceDriver.ActiveContextHandler
> >
> > 2. ContextActiveHandler classes
> > - DefaultContextActiveHandler
> > - BroadcastDriver.ContextActiveHandler
> > - DefaultDriverRestartContextActiveHandler
> > - LineCounter.ContextActiveHandler
> >
> > The same situations observed in the other classes like
> TaskCompletedHandler
> > and CompletedTaskHandler, too.
> >
> > I'm not quite sure if there is any reason for that. But, if there is no
> > reason, what about making a policy in REEF for naming of EventHandler
> class
> > before upcoming release? I think it's an easy fix.
> >
> > Regards,
> > Dongjoon.
>
Re: EventHandler class naming consistency policy?
Posted by Markus Weimer <ma...@weimo.de>.
Hi,
as with most things naming, the situation can be explained
historically. Initially `RunningTaskHandler` sounded more appealing.
But over time, we realized that `TaskRunningHandler` makes it much
easier to control-space into the names. If it wouldn't break so much
code, I'd even be in favor of renaming events that way, e.g.
`RunningTask` would become `TaskRunningEvent` or such for the same
reason.
That said, the confusing cases we have in the code right now seem to
be in examples and tests only. That should indeed be easy to fix, as
we don't promise compatibility between releases for that.
Markus
On Sun, Sep 27, 2015 at 11:28 PM, Dongjoon Hyun <do...@apache.org> wrote:
> Hi, all.
>
> Today, I found 42 ActiveContextHandler usages and 42 ContextActiveHandler
> usage in REEF. Of course, I know this is just a developer's choice in
> general. But, as an Apache Top-level project project, I hope REEF shows
> more consistent codes for new developers.
>
> For example, in the following REEF document,
> http://reef.apache.org/apidocs/0.12.0-incubating/org/apache/reef/wake/EventHandler.html
>
> 1. ActiveContextHandler classes
> - JobDriver.ActiveContextHandler
> - FailDriverDelayedMsg.ActiveContextHandler
> - JobDriver.DriverRestartActiveContextHandler
> - OutputServiceDriver.ActiveContextHandler
>
> 2. ContextActiveHandler classes
> - DefaultContextActiveHandler
> - BroadcastDriver.ContextActiveHandler
> - DefaultDriverRestartContextActiveHandler
> - LineCounter.ContextActiveHandler
>
> The same situations observed in the other classes like TaskCompletedHandler
> and CompletedTaskHandler, too.
>
> I'm not quite sure if there is any reason for that. But, if there is no
> reason, what about making a policy in REEF for naming of EventHandler class
> before upcoming release? I think it's an easy fix.
>
> Regards,
> Dongjoon.