You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@opennlp.apache.org by Jeffrey Zemerick <jz...@apache.org> on 2016/06/27 21:57:05 UTC
AdaptiveFeatureGenerator and FeatureGeneratorAdapter
Hi all,
Under the package opennlp.tools.util.featuregen there is an
interface AdaptiveFeatureGenerator and an abstract
class FeatureGeneratorAdapter. The interface defines
the createFeatures(), updateAdaptiveData(), and clearAdaptiveData()
methods. The abstract class implements this interface to provide default
implementations of the updateAdaptiveData() and clearAdaptiveData()
functions. Feature generators then either implement the interface or extend
the abstract class.
I created a patch to refactor these classes to remove the interface and use
the abstract class. (My motivation was I kept getting
AdaptiveFeatureGenerator and FeatureGeneratorAdapter confused due to their
similar naming and the inconsistency of feature generators either
implementing or extending.) The project does build and test with the patch
applied.
If you think this is a worthwhile change I'll submit it on JIRA. If not, no
problem and I'll work on not being confused. (Or if there's a reason for
both the interface and the abstract class that I'm not aware of please let
me know!)
Thanks,
Jeff
Re: AdaptiveFeatureGenerator and FeatureGeneratorAdapter
Posted by Jeffrey Zemerick <jz...@apache.org>.
Hi Jörn,
Yes, it would break backward compatibility. I will take your advice and try
that. Thanks!
Jeff
On Mon, Jul 4, 2016 at 5:53 AM, Joern Kottmann <ko...@gmail.com> wrote:
> Hello,
>
> as far as I understand the proposed change will break backward
> compatibility for users who implement AdaptiveFeatureGenerator.
> Is that correct?
>
> Anyway, I always like the idea of making things simpler. In Java 8 it is
> possible to declare default methods in an interface.
> http://docs.oracle.com/javase/tutorial/java/IandI/defaultmethods.html
>
> This can probably be done without breaking backward compatibility and then
> we will mark Feature Generator Adapter as deprecated so it can be removed
> one day.
>
> Would be nice if you could open an issue for it.
>
> Jörn
>
> On Mon, Jun 27, 2016 at 11:57 PM, Jeffrey Zemerick <jz...@apache.org>
> wrote:
>
> > Hi all,
> >
> > Under the package opennlp.tools.util.featuregen there is an
> > interface AdaptiveFeatureGenerator and an abstract
> > class FeatureGeneratorAdapter. The interface defines
> > the createFeatures(), updateAdaptiveData(), and clearAdaptiveData()
> > methods. The abstract class implements this interface to provide default
> > implementations of the updateAdaptiveData() and clearAdaptiveData()
> > functions. Feature generators then either implement the interface or
> extend
> > the abstract class.
> >
> > I created a patch to refactor these classes to remove the interface and
> use
> > the abstract class. (My motivation was I kept getting
> > AdaptiveFeatureGenerator and FeatureGeneratorAdapter confused due to
> their
> > similar naming and the inconsistency of feature generators either
> > implementing or extending.) The project does build and test with the
> patch
> > applied.
> >
> > If you think this is a worthwhile change I'll submit it on JIRA. If not,
> no
> > problem and I'll work on not being confused. (Or if there's a reason for
> > both the interface and the abstract class that I'm not aware of please
> let
> > me know!)
> >
> > Thanks,
> > Jeff
> >
>
Re: AdaptiveFeatureGenerator and FeatureGeneratorAdapter
Posted by Joern Kottmann <ko...@gmail.com>.
Hello,
as far as I understand the proposed change will break backward
compatibility for users who implement AdaptiveFeatureGenerator.
Is that correct?
Anyway, I always like the idea of making things simpler. In Java 8 it is
possible to declare default methods in an interface.
http://docs.oracle.com/javase/tutorial/java/IandI/defaultmethods.html
This can probably be done without breaking backward compatibility and then
we will mark Feature Generator Adapter as deprecated so it can be removed
one day.
Would be nice if you could open an issue for it.
Jörn
On Mon, Jun 27, 2016 at 11:57 PM, Jeffrey Zemerick <jz...@apache.org>
wrote:
> Hi all,
>
> Under the package opennlp.tools.util.featuregen there is an
> interface AdaptiveFeatureGenerator and an abstract
> class FeatureGeneratorAdapter. The interface defines
> the createFeatures(), updateAdaptiveData(), and clearAdaptiveData()
> methods. The abstract class implements this interface to provide default
> implementations of the updateAdaptiveData() and clearAdaptiveData()
> functions. Feature generators then either implement the interface or extend
> the abstract class.
>
> I created a patch to refactor these classes to remove the interface and use
> the abstract class. (My motivation was I kept getting
> AdaptiveFeatureGenerator and FeatureGeneratorAdapter confused due to their
> similar naming and the inconsistency of feature generators either
> implementing or extending.) The project does build and test with the patch
> applied.
>
> If you think this is a worthwhile change I'll submit it on JIRA. If not, no
> problem and I'll work on not being confused. (Or if there's a reason for
> both the interface and the abstract class that I'm not aware of please let
> me know!)
>
> Thanks,
> Jeff
>