Hello.

Le lun. 28 févr. 2022 à 07:11, Avijit Basak <avijit.ba...@gmail.com> a écrit :
>
> Hi All
>
>         Please see my comments below.
>
> > [...]
> >I just had a very quick look.
> >IIUC, you always provide "convenience" methods (e.g. the various
> >signatures for the "evolve" functionality).
> >Prior to merging into "master", we should simplify and limit the
> >discussion to the core functionality, i.e. not try and make decisions
> >for the user (like default values, ...).  Please keep the API as simple
> >as possible
> -- I have removed the mentioned evolve method.
> However, I had to catch two checked exceptions (InterruptedException,
> ExecutionException) and rethrow them. As of now I have handled them using
> the GeneticIllegalArgumentException. I think we need to introduce another
> exception class to handle this. Please share your thought regarding this.

I don't think that it's the right way to go; instantiating an "ExecutorService"
belongs to the GA application, not the GA library (whose relevant classes
need "only" be thread-safe).
There is some misunderstanding to be clarified in a dedicated discussion
(please file a new JIRA ticket).

Side note: Conflicts and duplicate commits have accumulated in the
dedicated "feature__MATH-1563__genetic_algorithm" branch.
I did not know how to proceed in order to avoid ending up with a messy
history in "master"; so I created a new branch (with the same name) with
all the new GA-related files added as a single commit.

Currently, this branch (based on your PR #205) fails the default goal,
because of a CheckStyle issue.  You shoudl always check locally that
running "mvn" without arguments does not generate any errors.

I also noticed that classes in "examples-ga" use "forbidden" library
classes: "GeneticIllegalArgumentException" is an "internal" class; we
must not advertize such classes in the example applications.
In general, it seems that "examples-ga" contains several classes and
methods that do not need to be "public".  This is especially true for
classes like "MathFunction" and "Coordinate".  [Having those "private"
helps users to tell what is part of the library's functionality from what is
just "dummy" placeholder code.]

Finally (for now), I've just noticed that there exist several classes named
"MathFunction", with same implementation!
Code duplication must be avoided, especially where we purport to display
best practices.
The various "Standalone" classes also look quite similar; consolidating the
"examples-ga" module (including full Javadoc) is necessary.  I still don't
understand why there are "...-legacy" modules in module "examples-ga".
If you want to offer the option of running the "old" implementation, you
could add a "legacy" flag (as "@Option" in the "Standalone" application).

Please use the new branch for all these ("cleanup") changes, as the basis
a PR (with a *single* commit).  Thanks.

Regards,
Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to