+1 even JVM hotspot optimizer does not like long functions. Smaller functions 
are more likely to be compiled and lead to performance gains.

It may be a good initial exercise for new contributors as well assuming that it 
will require proper unit test code coverage. 



Thank you,
Vlad

Отправлено с iPhone

> On Nov 4, 2015, at 11:36, Sandesh Hegde <[email protected]> wrote:
> 
> -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.
>> 

Reply via email to