-1 Going forward we can have the restriction on the function size. If the
reason for refactoring is 'hard to read', it is not a strong enough reason
to refactor a core piece.
If there is some issue in that part of the code and it is hard to debug
then it is fine.
On Wed, Nov 4, 2015 at 11:16 AM Pramod Immaneni <[email protected]>
wrote:
> +1 as long as it doesn't come at the expense of performance where it is
> critical like data flow.
>
> Thanks
>
> On Wed, Nov 4, 2015 at 10:42 AM, Timothy Farkas <[email protected]>
> wrote:
>
> > +1
> >
> > On Wed, Nov 4, 2015 at 10:38 AM, Chandni Singh <[email protected]>
> > wrote:
> >
> > > +1 for avoiding large monolithic functions and I think if the effort is
> > > already done to break an existing hard-to-understand function then we
> > > should merge that change (provided it passes all the checks).
> > >
> > > I think writing functions that are not monolithic cannot be enforced by
> > any
> > > tool. We can create a document that lays down certain guide lines but
> > other
> > > than that this needs to be part of code review process.
> > >
> > > Thanks,
> > > Chandni
> > >
> > >
> > > On Wed, Nov 4, 2015 at 10:16 AM, Ganelin, Ilya <
> > > [email protected]>
> > > wrote:
> > >
> > > > Hello all – in doing some refactoring of the code base, I’ve noticed
> > that
> > > > there are several large functions (several screens in length) with
> > > numerous
> > > > nested calls (as many as five deep). While there are no explicit
> > > guidelines
> > > > in the code style guide or in other common guidelines, it seems to be
> > > good
> > > > programming practice to avoid large monolithic functions since these
> > > become
> > > > harder to analyze and maintain. To that end, I submitted a PR to
> > refactor
> > > > one such function but this triggered discussion as to whether it’s
> > > > warranted to do this in the first place. Would love to get people’s
> > > > thoughts as to whether this is a good idea and if so, how could we
> > update
> > > > the style guide to reflect this requirement?
> > > >
> > > > The PR is below:
> > > >
> > > >
> > >
> >
> https://github.com/apache/incubator-apex-core/pull/129#issuecomment-153807144
> > > > ________________________________________________________
> > > >
> > > > The information contained in this e-mail is confidential and/or
> > > > proprietary to Capital One and/or its affiliates and may only be used
> > > > solely in performance of work or services for Capital One. The
> > > information
> > > > transmitted herewith is intended only for use by the individual or
> > entity
> > > > to which it is addressed. If the reader of this message is not the
> > > intended
> > > > recipient, you are hereby notified that any review, retransmission,
> > > > dissemination, distribution, copying or other use of, or taking of
> any
> > > > action in reliance upon this information is strictly prohibited. If
> you
> > > > have received this communication in error, please contact the sender
> > and
> > > > delete the material from your computer.
> > > >
> > >
> >
>
