[ 
https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12830233#action_12830233
 ] 

Ning Zhang commented on HIVE-1123:
----------------------------------

I think all these rules are good to make the coding style consistent, and 
making the code easier to understand by human once we are used to it. 

Sometimes I found our codebase is very "dense" and "compact" and it makes it's 
hard to understand at the first time. One particular case where I often find 
hard to understand is a long statement that can usually been broke up by 
several statements. One example is here:

{code}
    Operator op = putOpInsertMap(OperatorFactory.getAndMakeChild(
        new groupByDesc(mode, outputColumnNames, groupByKeys, aggregations,
            false), new RowSchema(groupByOutputRowResolver.getColumnInfos()),
        reduceSinkOperatorInfo), groupByOutputRowResolver);
{code}

I find it is easier to understand if we break it into several short statements 
with short comments. So when you browse the code, you can only skim the 
comments and understand what's going on.

It may not be the case that it can be done by style checking, but just want to 
see what others' opinions on this.

Another more coding style kind of thing is
{code}
     groupByKeys.add(new exprNodeColumnDesc(exprInfo.getType(), exprInfo
          .getInternalName(), exprInfo.getTabAlias(), exprInfo
          .getIsPartitionCol()));
{code}
vs.
{code}
     groupByKeys.add(
         new exprNodeColumnDesc(
             exprInfo.getType(), 
             exprInfo.getInternalName(), 
             exprInfo.getTabAlias(), 
             exprInfo.getIsPartitionCol()));
{code}

Currently some code using the former style. I found it is easier to understand 
using the second. Is there any convention for this?

> Checkstyle fixes
> ----------------
>
>                 Key: HIVE-1123
>                 URL: https://issues.apache.org/jira/browse/HIVE-1123
>             Project: Hadoop Hive
>          Issue Type: Task
>            Reporter: Carl Steinbach
>            Assignee: Carl Steinbach
>         Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, 
> HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, 
> HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, 
> HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, 
> HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, 
> HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, 
> HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, 
> HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to