You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by jaoki <gi...@git.apache.org> on 2015/11/15 19:44:17 UTC

[GitHub] flink pull request: [FLINK-3012] Refactor boilerplate code in Data...

GitHub user jaoki opened a pull request:

    https://github.com/apache/flink/pull/1355

    [FLINK-3012] Refactor boilerplate code in DataSourceTask

    Refactor repetitive and boilerplate code of while and if statements in DataSourceTask
    The previous if and else nest and to control the timing to instantiate 'reuse' and the new code emphasizes the point.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jaoki/flink refactor-DataSourceTask-around-calling-nextRecord

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/1355.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1355
    
----
commit a9c399920ada0147e6e787f8630a77ee3809f257
Author: jaoki <ja...@apache.org>
Date:   2015-11-15T18:34:07Z

    Refactor boilerplate code in DataSourceTask
    
    Refactor boilerplate code of while and if statements in DataSourceTask
    The previous if and else nest was to control the timing to instantiate 'reuse'.
    And the new code emphasizes the point.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-3012] Refactor boilerplate code in Data...

Posted by jaoki <gi...@git.apache.org>.
Github user jaoki commented on the pull request:

    https://github.com/apache/flink/pull/1355#issuecomment-156867821
  
    @StephanEwen , thank you for your feedback. Looping with an int variable is clearly wrong in Flink's use case, and I can change to long, but, I'd like to hear more of this refactor. I'd withdraw this PR if it is not what the community prefer at the first place. Let me know!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-3012] Refactor boilerplate code in Data...

Posted by jaoki <gi...@git.apache.org>.
Github user jaoki commented on the pull request:

    https://github.com/apache/flink/pull/1355#issuecomment-156946307
  
    Thank you for your input. @zentol ! Withdrawing PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-3012] Refactor boilerplate code in Data...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:

    https://github.com/apache/flink/pull/1355#issuecomment-156859098
  
    Thanks for the interest in helping us maintain clear code.
    
    I think actually that the original code was easier than the code with counter checks. Further more, it moves branches away from loops (easier optimizable by the JIT compiler). Finally, the new code will be wrong if the counter overflows/wraps around, which is not unthinkable. 4 Billion elements is quite within the scope of many applications.
    
    I am curious about other opinions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-3012] Refactor boilerplate code in Data...

Posted by jaoki <gi...@git.apache.org>.
Github user jaoki closed the pull request at:

    https://github.com/apache/flink/pull/1355


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-3012] Refactor boilerplate code in Data...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the pull request:

    https://github.com/apache/flink/pull/1355#issuecomment-156869041
  
    I'm not too fond of the new version either and feel like this code bit doesn't really need more clarity, simply because it isn't a lot of code. 
    
    Using a counter at all doesn't really make sense to me, you aren't counting anything effectively. a Boolean seems more intuitive. It is nit-picky but we are going for clarity here after all :)



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---