[jira] [Commented] (ZOOKEEPER-1094) Small improvements to LeaderElection and Vote classes

["Hadoop QA (JIRA)" <ji...@apache.org>]

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

Hadoop QA commented on ZOOKEEPER-1094:
--------------------------------------

+1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12482485/ZK-1094.patch
  against trunk revision 1135270.

    +1 @author.  The patch does not contain any @author tags.

    +1 tests included.  The patch appears to include 21 new or modified tests.

    +1 javadoc.  The javadoc tool did not generate any warning messages.

    +1 javac.  The applied patch does not increase the total number of javac compiler warnings.

    +1 findbugs.  The patch does not introduce any new Findbugs (version 1.3.9) warnings.

    +1 release audit.  The applied patch does not increase the total number of release audit warnings.

    +1 core tests.  The patch passed core unit tests.

    +1 contrib tests.  The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/314//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/314//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/314//console

This message is automatically generated.

> Small improvements to LeaderElection and Vote classes
> -----------------------------------------------------
>
>                 Key: ZOOKEEPER-1094
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1094
>             Project: ZooKeeper
>          Issue Type: Improvement
>          Components: quorum
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>            Priority: Minor
>         Attachments: ZK-1094.patch
>
>
> 1. o.a.z.q.Vote is a struct-style class, whose fields are public and not final. 
> In general, we should prefer making the fields of these kind of classes final, and hiding them behind getters for the following reasons:
> * Marking them as final allows clients of the class not to worry about any synchronisation when accessing the fields
> * Hiding them behind getters allows us to change the implementation of the class without changing the API. 
> Object creation is very cheap. It's ok to create new Votes rather than mutate existing ones. 
> 2. Votes are mainly used in the LeaderElection class. In this class a map of addresses to votes is passed in to countVotes, which modifies the map contents inside an iterator (and therefore changes the object passed in by reference). This is pretty gross, so at the same time I've slightly refactored this method to return information about the number of validVotes in the ElectionResult class, which is returned by countVotes. 
> 3. The previous implementation of countVotes was quadratic in the number of votes. It is possible to do this linearly. No real speed-up is expected as a result, but it salves the CS OCD in me :)

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira