[
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
