[ https://issues.apache.org/jira/browse/HDFS-4352?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13547613#comment-13547613 ]
Todd Lipcon commented on HDFS-4352: ----------------------------------- bq. I think Colin and I have discussed this issue for few recent days. Suresh also has joined the discussion at the end. Any more to add? Colin had left it with this question: bq. HDFS-4353 adds asserts to BlockReaderFactory#newBlockReader that check that all essential parameters are set. That is how you can know that you have set all the essential parameters. Does that address your concerns? ...which you didn't answer. So I think the discussion was still going on, and it's premature to revert until you explain why his proposed amendment to the patch doesn't satisfy you. I know that Suresh added his opinions as well -- so now we have 4 developers' opinions, 2 of whom think it's a good change and 2 of whom don't. All of us are experienced developers, and several of us have worked on the code in question for multiple years. Given that, I don't think this is cut and dry "good" or "bad", but rather a matter of opinion. Rather than closing the issue, let's come to a design we can all agree on. I think a Builder pattern cleans up the function substantially, and is used elsewhere in lots of places in Hadoop (eg RPC.Builder which Suresh reviewed and committed) > Encapsulate arguments to BlockReaderFactory in a class > ------------------------------------------------------ > > Key: HDFS-4352 > URL: https://issues.apache.org/jira/browse/HDFS-4352 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client > Affects Versions: 2.0.3-alpha > Reporter: Colin Patrick McCabe > Attachments: 01b.patch, 01.patch > > > Encapsulate the arguments to BlockReaderFactory in a class to avoid having to > pass around 10+ arguments to a few different functions. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira