hadoop-hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ning Zhang (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HIVE-1123) Checkstyle fixes
Date Fri, 05 Feb 2010 19:04:28 GMT

    [ 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.


Mime
View raw message