[
https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13437715#comment-13437715
]
Carl Steinbach commented on HIVE-3394:
--------------------------------------
@Tim: Thanks for taking time to look into this.
I recommended using the builder pattern because these constructors are
unreadable due to the large number of input parameters. Replacing a twenty-one
parameter constructor (i.e. CreateTableDesc()) with a builder object that takes
21 input parameters doesn't really solve the problem. Here's a quote from
chapter2 of Bloch's Effective Java book:
{quote}
Instead of making the desired object directly, the client calls a constructor
(or static factory) with all of the the *required parameters* and gets a
builder object. Then the client calls setter-like methods on the builder object
to set each *optional parameter* of interest. Finally, the client calls a
parameterless build() method to generate the object, which is *immutable*.
{quote}
Taking CreateTableDesc as an example, I think the only *required* parameters
are tableName and cols. Everything else should have a default value that can
optionally be overridden using a setter exposed by the Builder.
Another problem with many of these objects (including CreateTableDesc) is that
they should be immutable, but currently aren't. This is a consequence of the
fact that many of the getters return references to mutable instance variables.
For example, CreateTableDesc.getPartCols() returns a reference to the internal
list, which means that if I add an element to the list I actually modified the
internal state of the CreateTableDesc object. The workaround for this problem
is to return a copy of the internal list instead of the list itself.
Also, I saw a lot of raw types on the LHS (e.g. ArrayList instead of List).
> Refactor a few classes' constructors with creational patterns
> -------------------------------------------------------------
>
> Key: HIVE-3394
> URL: https://issues.apache.org/jira/browse/HIVE-3394
> Project: Hive
> Issue Type: Bug
> Reporter: Gang Tim Liu
> Assignee: Gang Tim Liu
> Priority: Minor
> Attachments: HIVE-3394.patch.1
>
>
> It's good to refactor the following classes' constructors with
> builder/factory pattern. This should be done before HIVE-3072 so that
> HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
