[jira] [Commented] (BEAM-134) Investigate use of AutoValue

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/BEAM-134?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15220778#comment-15220778 ] 

ASF GitHub Bot commented on BEAM-134:
-------------------------------------

GitHub user swegner opened a pull request:

    https://github.com/apache/incubator-beam/pull/109

    [BEAM-134] Example of AutoValue integration

    This is a sample for [\[BEAM-134\] Investigate use of AutoValue](https://issues.apache.org/jira/browse/BEAM-134)
    
    In the case of `IsmFormat`, there are four inner-classes used as immutable value types which can be simplified with AutoValue. Each of them demonstrate some interesting functionality:
    
    * `KeyPrefix` is the simplest of the bunch, and it's implementation basically disappears after conversion.
    * `IsmRecord` can act as two different types (value or metadata), and has validation logic in its getters they verify correct usage. In this case, we can keep the existing getter functionality and let AutoValue hook into separate package-private abstract properties. `IsmShard` is similar in this respect.
    * `Footer` provides has custom logic in its `.toString()` to include a version string. For other value classes, AutoValue will generate a `toString()` implementation compatible with the equivalent `Objects.toStringHelper` version. In this case, we can keep the existing implementation and AutoValue won't override it.
    
    As noted in the JIRA issue, I've identified 39 distinct classes which could be similarly converted. The benefits to converting are:
    * Less boilerplate code to maintain.
    * Eliminate a common source of bugs.
    * Lower the barrier to writing immutable value types, which carry their own benefits.
    
    I recommend we take this work.

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

    $ git pull https://github.com/swegner/incubator-beam autovalue-example

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

    https://github.com/apache/incubator-beam/pull/109.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 #109
    
----
commit 880a0685e3ff8e235cb9f046c4097e3f797e9760
Author: Scott Wegner <sw...@google.com>
Date:   2016-03-31T22:04:26Z

    Refactor IsmFormat value classes to use AutoValue

----


> Investigate use of AutoValue
> ----------------------------
>
>                 Key: BEAM-134
>                 URL: https://issues.apache.org/jira/browse/BEAM-134
>             Project: Beam
>          Issue Type: Improvement
>          Components: sdk-java-core
>            Reporter: Scott Wegner
>            Assignee: Scott Wegner
>            Priority: Minor
>         Attachments: 0001-Mark-classes-which-might-benefit-from-AutoValue.patch
>
>
> The initial PR for [BEAM-118] added a dependency on [AutoValue|https://github.com/google/auto/tree/master/value#how-to-use-autovalue] to auto-implement equality semantics for a new POJO. We decided to remove the dependency because the cost of adding the dependency for this feature may not be worth it for the value.
> However, we could use AutoValue for all of our POJO's, it might be worth it. The proposal here is to follow-up with an investigation on whether we would gain significant value to porting our code to use AutoValue instead of hand-written POJO's.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)