You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Vladimir Ozerov <vo...@gridgain.com> on 2017/05/16 10:01:07 UTC
Minimize amount of inner classes, predicates, tuples, etc
Igniters,
Ignite is known to be complex product. Partially this complexity comes from
... complex problems we are trying to resolve. But partially it comes from
how we write our code. I noticed several anti-patterns which makes our code
hard to manage:
1) Inner classes (both static and non-static)
2) Usage of generic tuples
3) Usage of many classes from our "lang"package - closures, predicates,
etc..
When combined these anti-patterns makes our classes huge, leaky in terms of
abstractions, hard to follow and refactor. I would like to propose to
restrict usages of these constructs as much as possible in non-test code.
Classes should be top-level by default, unless "inner" semantics are
absolutely necessary or saves a lot of code. Tuples and closures should be
replaced with concrete classes, specific to your case.
Thoughts?
Vladimir.
Re: Minimize amount of inner classes, predicates, tuples, etc
Posted by Dmitry Pavlov <dp...@gmail.com>.
Hi Vladimir,
Do you have any idea how this rules may be automatically controlled by, for
example, code inspections and/or TeamCity?
I do beleive if rule is controlled by some automatic way (not only by
argreement) it has a chance to become true practice in real life.
If this excellent idea remains only agreement on dev list, it may be
forgotten in the future.
Sincerely,
Dmitriy Pavlov
вт, 16 мая 2017 г. в 13:01, Vladimir Ozerov <vo...@gridgain.com>:
> Igniters,
>
> Ignite is known to be complex product. Partially this complexity comes from
> ... complex problems we are trying to resolve. But partially it comes from
> how we write our code. I noticed several anti-patterns which makes our code
> hard to manage:
>
> 1) Inner classes (both static and non-static)
> 2) Usage of generic tuples
> 3) Usage of many classes from our "lang"package - closures, predicates,
> etc..
>
> When combined these anti-patterns makes our classes huge, leaky in terms of
> abstractions, hard to follow and refactor. I would like to propose to
> restrict usages of these constructs as much as possible in non-test code.
> Classes should be top-level by default, unless "inner" semantics are
> absolutely necessary or saves a lot of code. Tuples and closures should be
> replaced with concrete classes, specific to your case.
>
> Thoughts?
>
> Vladimir.
>
Re: Minimize amount of inner classes, predicates, tuples, etc
Posted by ALEKSEY KUZNETSOV <al...@gmail.com>.
yeah, good proposal! In addition i would suggest start adding more javadocs
on current code
ср, 17 мая 2017 г. в 11:08, Vladimir Ozerov <vo...@gridgain.com>:
> Vyacheslav,
>
> When you analyze code full of generic stuff like tuples and closures, it is
> much harder to understand what is going on and navigate the code.
>
> Good coding: GridDhtAtomicCache - specific interface for callback. Easy to
> navigate and understand.
>
> interface UpdateReplyClosure extends
> CI2<GridNearAtomicAbstractUpdateRequest, GridNearAtomicUpdateResponse> {
> ...
> }
>
> Bad coding: CacheContinuousQueryHandler.AcknowledgeBuffer - overcomplicated
> generic, no JavaDocs, nullable semantics. Complexity out of nothing.
>
> @Nullable synchronized IgniteBiTuple<Map<Integer, Long>,
> Set<AffinityTopologyVersion>> onAcknowledged(GridContinuousBatch batch) {
> ...
> }
>
>
> On Tue, May 16, 2017 at 1:22 PM, Vyacheslav Daradur <da...@gmail.com>
> wrote:
>
> > Vladimir, thank you for this suggestion.
> > As newcomer in the Ignite I agree with you, sometimes it is very hard to
> > understand huge classes which contain sets of inner classes.
> >
> > >> 1) Inner classes (both static and non-static)
> > Completelly agree, especially classes which will be serialized.
> >
> > >> 2) Usage of generic tuples
> > Why or in what cases?
> >
> > >> Tuples and closures should be replaced with concrete classes, specific
> > to your case.
> > Can you give an example?
> >
> > 2017-05-16 13:01 GMT+03:00 Vladimir Ozerov <vo...@gridgain.com>:
> >
> > > Igniters,
> > >
> > > Ignite is known to be complex product. Partially this complexity comes
> > from
> > > ... complex problems we are trying to resolve. But partially it comes
> > from
> > > how we write our code. I noticed several anti-patterns which makes our
> > code
> > > hard to manage:
> > >
> > > 1) Inner classes (both static and non-static)
> > > 2) Usage of generic tuples
> > > 3) Usage of many classes from our "lang"package - closures, predicates,
> > > etc..
> > >
> > > When combined these anti-patterns makes our classes huge, leaky in
> terms
> > of
> > > abstractions, hard to follow and refactor. I would like to propose to
> > > restrict usages of these constructs as much as possible in non-test
> code.
> > > Classes should be top-level by default, unless "inner" semantics are
> > > absolutely necessary or saves a lot of code. Tuples and closures should
> > be
> > > replaced with concrete classes, specific to your case.
> > >
> > > Thoughts?
> > >
> > > Vladimir.
> > >
> >
> >
> >
> > --
> > Best Regards, Vyacheslav
> >
>
--
*Best Regards,*
*Kuznetsov Aleksey*
Re: Minimize amount of inner classes, predicates, tuples, etc
Posted by Vyacheslav Daradur <da...@gmail.com>.
Vladimir,
Thank you for explanation and examples, it is clearly now.
Maybe it will be great to add your suggestions with explanation and
examples to the article[1] "Coding guidelines"
[1] https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines
2017-05-17 11:08 GMT+03:00 Vladimir Ozerov <vo...@gridgain.com>:
> Vyacheslav,
>
> When you analyze code full of generic stuff like tuples and closures, it is
> much harder to understand what is going on and navigate the code.
>
> Good coding: GridDhtAtomicCache - specific interface for callback. Easy to
> navigate and understand.
>
> interface UpdateReplyClosure extends
> CI2<GridNearAtomicAbstractUpdateRequest, GridNearAtomicUpdateResponse> {
> ...
> }
>
> Bad coding: CacheContinuousQueryHandler.AcknowledgeBuffer -
> overcomplicated
> generic, no JavaDocs, nullable semantics. Complexity out of nothing.
>
> @Nullable synchronized IgniteBiTuple<Map<Integer, Long>,
> Set<AffinityTopologyVersion>> onAcknowledged(GridContinuousBatch batch) {
> ...
> }
>
>
> On Tue, May 16, 2017 at 1:22 PM, Vyacheslav Daradur <da...@gmail.com>
> wrote:
>
> > Vladimir, thank you for this suggestion.
> > As newcomer in the Ignite I agree with you, sometimes it is very hard to
> > understand huge classes which contain sets of inner classes.
> >
> > >> 1) Inner classes (both static and non-static)
> > Completelly agree, especially classes which will be serialized.
> >
> > >> 2) Usage of generic tuples
> > Why or in what cases?
> >
> > >> Tuples and closures should be replaced with concrete classes, specific
> > to your case.
> > Can you give an example?
> >
> > 2017-05-16 13:01 GMT+03:00 Vladimir Ozerov <vo...@gridgain.com>:
> >
> > > Igniters,
> > >
> > > Ignite is known to be complex product. Partially this complexity comes
> > from
> > > ... complex problems we are trying to resolve. But partially it comes
> > from
> > > how we write our code. I noticed several anti-patterns which makes our
> > code
> > > hard to manage:
> > >
> > > 1) Inner classes (both static and non-static)
> > > 2) Usage of generic tuples
> > > 3) Usage of many classes from our "lang"package - closures, predicates,
> > > etc..
> > >
> > > When combined these anti-patterns makes our classes huge, leaky in
> terms
> > of
> > > abstractions, hard to follow and refactor. I would like to propose to
> > > restrict usages of these constructs as much as possible in non-test
> code.
> > > Classes should be top-level by default, unless "inner" semantics are
> > > absolutely necessary or saves a lot of code. Tuples and closures should
> > be
> > > replaced with concrete classes, specific to your case.
> > >
> > > Thoughts?
> > >
> > > Vladimir.
> > >
> >
> >
> >
> > --
> > Best Regards, Vyacheslav
> >
>
--
Best Regards, Vyacheslav
Re: Minimize amount of inner classes, predicates, tuples, etc
Posted by Vladimir Ozerov <vo...@gridgain.com>.
Vyacheslav,
When you analyze code full of generic stuff like tuples and closures, it is
much harder to understand what is going on and navigate the code.
Good coding: GridDhtAtomicCache - specific interface for callback. Easy to
navigate and understand.
interface UpdateReplyClosure extends
CI2<GridNearAtomicAbstractUpdateRequest, GridNearAtomicUpdateResponse> {
...
}
Bad coding: CacheContinuousQueryHandler.AcknowledgeBuffer - overcomplicated
generic, no JavaDocs, nullable semantics. Complexity out of nothing.
@Nullable synchronized IgniteBiTuple<Map<Integer, Long>,
Set<AffinityTopologyVersion>> onAcknowledged(GridContinuousBatch batch) {
...
}
On Tue, May 16, 2017 at 1:22 PM, Vyacheslav Daradur <da...@gmail.com>
wrote:
> Vladimir, thank you for this suggestion.
> As newcomer in the Ignite I agree with you, sometimes it is very hard to
> understand huge classes which contain sets of inner classes.
>
> >> 1) Inner classes (both static and non-static)
> Completelly agree, especially classes which will be serialized.
>
> >> 2) Usage of generic tuples
> Why or in what cases?
>
> >> Tuples and closures should be replaced with concrete classes, specific
> to your case.
> Can you give an example?
>
> 2017-05-16 13:01 GMT+03:00 Vladimir Ozerov <vo...@gridgain.com>:
>
> > Igniters,
> >
> > Ignite is known to be complex product. Partially this complexity comes
> from
> > ... complex problems we are trying to resolve. But partially it comes
> from
> > how we write our code. I noticed several anti-patterns which makes our
> code
> > hard to manage:
> >
> > 1) Inner classes (both static and non-static)
> > 2) Usage of generic tuples
> > 3) Usage of many classes from our "lang"package - closures, predicates,
> > etc..
> >
> > When combined these anti-patterns makes our classes huge, leaky in terms
> of
> > abstractions, hard to follow and refactor. I would like to propose to
> > restrict usages of these constructs as much as possible in non-test code.
> > Classes should be top-level by default, unless "inner" semantics are
> > absolutely necessary or saves a lot of code. Tuples and closures should
> be
> > replaced with concrete classes, specific to your case.
> >
> > Thoughts?
> >
> > Vladimir.
> >
>
>
>
> --
> Best Regards, Vyacheslav
>
Re: Minimize amount of inner classes, predicates, tuples, etc
Posted by Vyacheslav Daradur <da...@gmail.com>.
Vladimir, thank you for this suggestion.
As newcomer in the Ignite I agree with you, sometimes it is very hard to
understand huge classes which contain sets of inner classes.
>> 1) Inner classes (both static and non-static)
Completelly agree, especially classes which will be serialized.
>> 2) Usage of generic tuples
Why or in what cases?
>> Tuples and closures should be replaced with concrete classes, specific
to your case.
Can you give an example?
2017-05-16 13:01 GMT+03:00 Vladimir Ozerov <vo...@gridgain.com>:
> Igniters,
>
> Ignite is known to be complex product. Partially this complexity comes from
> ... complex problems we are trying to resolve. But partially it comes from
> how we write our code. I noticed several anti-patterns which makes our code
> hard to manage:
>
> 1) Inner classes (both static and non-static)
> 2) Usage of generic tuples
> 3) Usage of many classes from our "lang"package - closures, predicates,
> etc..
>
> When combined these anti-patterns makes our classes huge, leaky in terms of
> abstractions, hard to follow and refactor. I would like to propose to
> restrict usages of these constructs as much as possible in non-test code.
> Classes should be top-level by default, unless "inner" semantics are
> absolutely necessary or saves a lot of code. Tuples and closures should be
> replaced with concrete classes, specific to your case.
>
> Thoughts?
>
> Vladimir.
>
--
Best Regards, Vyacheslav