Utility classes

[Sergey Evdokimov]

Hello,

I have some suggestions for improvement utility classes.

1.) We have big class org.apache.ignite.internal.util.lang.GridFunc that
has a lot of methods in 8000 lines. I propose to split it to several
classes where utility methods are group by purpose. For example methods to
work with IgnitePredicate should be in IgnitePredicates class, methods to
work with Iterator should be in IgniteIterators class, etc... Same grouping
is used in Guava.

2.) Methods like GridFunc.not(IgnitePredicate p) must not return anonymous
class, it should return class with correct name. When I debug code and see
to value of variable I cannot understand that is "GridFunc$123", something
like "GridFunc$PredicateNot" looks much better.

3.) Methods with ugly signature like GridFunc.iterator(Iterable c,
IgniteClosure trans, boolean readOnly, @Nullable IgnitePredicate<? super
T1>... p) should be split to simple methods like
"GridFunc.transform(Iterable itr, IgniteClosure trans)",
 GridFunc.filter(Iterable itr, IgnitePredicate p),
GridFunc.readonly(Iterable c)

Thoughts?

[dsetrakyan]

On Thu, May 7, 2015 at 8:54 AM, Sergey Evdokimov <[hidden email]>
wrote:

> Hello,
>
> I have some suggestions for improvement utility classes.
>
> 1.) We have big class org.apache.ignite.internal.util.lang.GridFunc that
> has a lot of methods in 8000 lines. I propose to split it to several
> classes where utility methods are group by purpose. For example methods to
> work with IgnitePredicate should be in IgnitePredicates class, methods to
> work with Iterator should be in IgniteIterators class, etc... Same grouping
> is used in Guava.
>
> 2.) Methods like GridFunc.not(IgnitePredicate p) must not return anonymous
> class, it should return class with correct name. When I debug code and see
> to value of variable I cannot understand that is "GridFunc$123", something
> like "GridFunc$PredicateNot" looks much better.
>
> 3.) Methods with ugly signature like GridFunc.iterator(Iterable c,
> IgniteClosure trans, boolean readOnly, @Nullable IgnitePredicate<? super
> T1>... p) should be split to simple methods like
> "GridFunc.transform(Iterable itr, IgniteClosure trans)",
>  GridFunc.filter(Iterable itr, IgnitePredicate p),
> GridFunc.readonly(Iterable c)
>
> Thoughts?
>

GridFunc is a legacy class that should be deprecated in my view. It comes
from the previous life when we were adding functional abstracts to Java.
Obviously, with Java 8 it is not needed.

My suggestion would be to start by cleaning up GridFunc and removing all
the methods that are not currently used. Then we can see how we can
refactor the methods that are in use.

D.

[Alexey Kuznetsov-2]

I like Sergey suggestions.

On Thu, May 7, 2015 at 10:54 PM, Sergey Evdokimov <[hidden email]>
wrote:

> Hello,
>
> I have some suggestions for improvement utility classes.
>
> 1.) We have big class org.apache.ignite.internal.util.lang.GridFunc that
> has a lot of methods in 8000 lines. I propose to split it to several
> classes where utility methods are group by purpose. For example methods to
> work with IgnitePredicate should be in IgnitePredicates class, methods to
> work with Iterator should be in IgniteIterators class, etc... Same grouping
> is used in Guava.
>
> 2.) Methods like GridFunc.not(IgnitePredicate p) must not return anonymous
> class, it should return class with correct name. When I debug code and see
> to value of variable I cannot understand that is "GridFunc$123", something
> like "GridFunc$PredicateNot" looks much better.
>
> 3.) Methods with ugly signature like GridFunc.iterator(Iterable c,
> IgniteClosure trans, boolean readOnly, @Nullable IgnitePredicate<? super
> T1>... p) should be split to simple methods like
> "GridFunc.transform(Iterable itr, IgniteClosure trans)",
>  GridFunc.filter(Iterable itr, IgnitePredicate p),
> GridFunc.readonly(Iterable c)
>
> Thoughts?
>

--
Alexey Kuznetsov
GridGain Systems
www.gridgain.com

[Sergey Evdokimov]

I removed unused methods from GridFunc, size of GridFunc became 4500 line
instead of 8000 line.

On Thu, May 7, 2015 at 8:13 PM, Alexey Kuznetsov <[hidden email]>
wrote:

> I like Sergey suggestions.
>
> On Thu, May 7, 2015 at 10:54 PM, Sergey Evdokimov <[hidden email]
> >
> wrote:
>
> > Hello,
> >
> > I have some suggestions for improvement utility classes.
> >
> > 1.) We have big class org.apache.ignite.internal.util.lang.GridFunc that
> > has a lot of methods in 8000 lines. I propose to split it to several
> > classes where utility methods are group by purpose. For example methods
> to
> > work with IgnitePredicate should be in IgnitePredicates class, methods to
> > work with Iterator should be in IgniteIterators class, etc... Same
> grouping
> > is used in Guava.
> >
> > 2.) Methods like GridFunc.not(IgnitePredicate p) must not return
> anonymous
> > class, it should return class with correct name. When I debug code and
> see
> > to value of variable I cannot understand that is "GridFunc$123",
> something
> > like "GridFunc$PredicateNot" looks much better.
> >
> > 3.) Methods with ugly signature like GridFunc.iterator(Iterable c,
> > IgniteClosure trans, boolean readOnly, @Nullable IgnitePredicate<? super
> > T1>... p) should be split to simple methods like
> > "GridFunc.transform(Iterable itr, IgniteClosure trans)",
> >  GridFunc.filter(Iterable itr, IgnitePredicate p),
> > GridFunc.readonly(Iterable c)
> >
> > Thoughts?
> >
>
>
>
> --
> Alexey Kuznetsov
> GridGain Systems
> www.gridgain.com
>

[Konstantin Boudnik-2]

On Thu, May 07, 2015 at 08:53PM, Sergey Evdokimov wrote:
> I removed unused methods from GridFunc, size of GridFunc became 4500 line
> instead of 8000 line.

Still sounds like an awful lot: perhaps the split refactoring is a good way to
deal with it.

>
> On Thu, May 7, 2015 at 8:13 PM, Alexey Kuznetsov <[hidden email]>
> wrote:
>
> > I like Sergey suggestions.
> >
> > On Thu, May 7, 2015 at 10:54 PM, Sergey Evdokimov <[hidden email]
> > >
> > wrote:
> >
> > > Hello,
> > >
> > > I have some suggestions for improvement utility classes.
> > >
> > > 1.) We have big class org.apache.ignite.internal.util.lang.GridFunc that
> > > has a lot of methods in 8000 lines. I propose to split it to several
> > > classes where utility methods are group by purpose. For example methods
> > to
> > > work with IgnitePredicate should be in IgnitePredicates class, methods to
> > > work with Iterator should be in IgniteIterators class, etc... Same
> > grouping
> > > is used in Guava.
> > >
> > > 2.) Methods like GridFunc.not(IgnitePredicate p) must not return
> > anonymous
> > > class, it should return class with correct name. When I debug code and
> > see
> > > to value of variable I cannot understand that is "GridFunc$123",
> > something
> > > like "GridFunc$PredicateNot" looks much better.
> > >
> > > 3.) Methods with ugly signature like GridFunc.iterator(Iterable c,
> > > IgniteClosure trans, boolean readOnly, @Nullable IgnitePredicate<? super
> > > T1>... p) should be split to simple methods like
> > > "GridFunc.transform(Iterable itr, IgniteClosure trans)",
> > >  GridFunc.filter(Iterable itr, IgnitePredicate p),
> > > GridFunc.readonly(Iterable c)
> > >
> > > Thoughts?
> > >
> >
> >
> >
> > --
> > Alexey Kuznetsov
> > GridGain Systems
> > www.gridgain.com
> >

[Konstantin Boudnik-2]

On Thu, May 07, 2015 at 09:32AM, Dmitriy Setrakyan wrote:

> On Thu, May 7, 2015 at 8:54 AM, Sergey Evdokimov <[hidden email]>
> wrote:
>
> > Hello,
> >
> > I have some suggestions for improvement utility classes.
> >
> > 1.) We have big class org.apache.ignite.internal.util.lang.GridFunc that
> > has a lot of methods in 8000 lines. I propose to split it to several
> > classes where utility methods are group by purpose. For example methods to
> > work with IgnitePredicate should be in IgnitePredicates class, methods to
> > work with Iterator should be in IgniteIterators class, etc... Same grouping
> > is used in Guava.
> >
> > 2.) Methods like GridFunc.not(IgnitePredicate p) must not return anonymous
> > class, it should return class with correct name. When I debug code and see
> > to value of variable I cannot understand that is "GridFunc$123", something
> > like "GridFunc$PredicateNot" looks much better.
> >
> > 3.) Methods with ugly signature like GridFunc.iterator(Iterable c,
> > IgniteClosure trans, boolean readOnly, @Nullable IgnitePredicate<? super
> > T1>... p) should be split to simple methods like
> > "GridFunc.transform(Iterable itr, IgniteClosure trans)",
> >  GridFunc.filter(Iterable itr, IgnitePredicate p),
> > GridFunc.readonly(Iterable c)
> >
> > Thoughts?
> >
>
> GridFunc is a legacy class that should be deprecated in my view. It comes
> from the previous life when we were adding functional abstracts to Java.
> Obviously, with Java 8 it is not needed.

Which brings an interesting question: shall we plan for dropping Java7 support
as it was OEL'ed?

> My suggestion would be to start by cleaning up GridFunc and removing all
> the methods that are not currently used. Then we can see how we can
> refactor the methods that are in use.
>
> D.

[nivanov]

I was the one implementing the FP approach to original GridGain APIs. As
Dmitriy mentioned - with Java 8 a lot of this is no longer necessary or
desirable. Furthermore, in retrospect trying to mimic FP style in non-FP
Java was a futile exercise and led to some APIs becoming unwieldy. I think
we can safely deprecate (most of) this over time.

For those looking for FP-style APIs:
- either use Java 8 rudimentary support,
- or use Groovy for slightly better FP support,
- or use Clojure or Scala for a full FP support.

All languages above can natively use all Ignite APIs. For example, in Scala
only a small set of implicit conversions is required to have pretty fluid
FP usage pattern (look at Scalar DSL for more examples).

--
Nikita Ivanov
​GridGain founder​



On Thu, May 7, 2015 at 11:58 AM, Konstantin Boudnik <[hidden email]> wrote:

> On Thu, May 07, 2015 at 09:32AM, Dmitriy Setrakyan wrote:
> > On Thu, May 7, 2015 at 8:54 AM, Sergey Evdokimov <
> [hidden email]>
> > wrote:
> >
> > > Hello,
> > >
> > > I have some suggestions for improvement utility classes.
> > >
> > > 1.) We have big class org.apache.ignite.internal.util.lang.GridFunc
> that
> > > has a lot of methods in 8000 lines. I propose to split it to several
> > > classes where utility methods are group by purpose. For example
> methods to
> > > work with IgnitePredicate should be in IgnitePredicates class, methods
> to
> > > work with Iterator should be in IgniteIterators class, etc... Same
> grouping
> > > is used in Guava.
> > >
> > > 2.) Methods like GridFunc.not(IgnitePredicate p) must not return
> anonymous
> > > class, it should return class with correct name. When I debug code and
> see
> > > to value of variable I cannot understand that is "GridFunc$123",
> something
> > > like "GridFunc$PredicateNot" looks much better.
> > >
> > > 3.) Methods with ugly signature like GridFunc.iterator(Iterable c,
> > > IgniteClosure trans, boolean readOnly, @Nullable IgnitePredicate<?
> super
> > > T1>... p) should be split to simple methods like
> > > "GridFunc.transform(Iterable itr, IgniteClosure trans)",
> > >  GridFunc.filter(Iterable itr, IgnitePredicate p),
> > > GridFunc.readonly(Iterable c)
> > >
> > > Thoughts?
> > >
> >
> > GridFunc is a legacy class that should be deprecated in my view. It comes
> > from the previous life when we were adding functional abstracts to Java.
> > Obviously, with Java 8 it is not needed.
>
> Which brings an interesting question: shall we plan for dropping Java7
> support
> as it was OEL'ed?
>
> > My suggestion would be to start by cleaning up GridFunc and removing all
> > the methods that are not currently used. Then we can see how we can
> > refactor the methods that are in use.
> >
> > D.
>