hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ashutosh Chauhan (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HIVE-5520) Use factory methods to instantiate HiveDecimal instead of constructors
Date Sat, 12 Oct 2013 04:35:43 GMT

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

Ashutosh Chauhan commented on HIVE-5520:
----------------------------------------

All the ctors which were throwing exceptions earlier {{ throw new NumberFormatException("Assignment
would result in truncation"); }} are now silently returning null in those cases, resulting
in almost all mathematical functions like divide (), setScale(), remainder() etc to return
null in error conditions instead of throwing exception. Is that desirable ? e.g,, I think
if divide () fails on BigDecimal that should throw exception instead of returning null.

> Use factory methods to instantiate HiveDecimal instead of constructors
> ----------------------------------------------------------------------
>
>                 Key: HIVE-5520
>                 URL: https://issues.apache.org/jira/browse/HIVE-5520
>             Project: Hive
>          Issue Type: Improvement
>          Components: Types
>    Affects Versions: 0.11.0
>            Reporter: Xuefu Zhang
>            Assignee: Xuefu Zhang
>             Fix For: 0.13.0
>
>         Attachments: HIVE-5520.1.patch, HIVE-5520.patch
>
>
> Currently HiveDecimal class provided a bunch of constructors that  unfortunately also
throws a runtime exception. For example,
> {code}
>  public HiveDecimal(BigInteger unscaled, int scale) {
>     bd = this.normalize(new BigDecimal(unscaled, scale), MAX_PRECISION, false);
>     if (bd == null) {
>      throw new NumberFormatException("Assignment would result in truncation");
>    }
> {code}
> As a result, it's hard for the caller to detect error occurrences and the error handling
is also complicated. In many cases, the error handling is omitted or missed. For instance,
> {code}
>          HiveDecimalWritable result = new HiveDecimalWritable(HiveDecimal.ZERO);
>         try {
>           result.set(aggregation.sum.divide(new HiveDecimal(aggregation.count)));
>         } catch (NumberFormatException e) {
>           result = null;
>         }
> {code} 
> Throwing runtime exception while expecting caller to catch seems anti-pattern. In the
case of constructor, factory class or methods seem more appropriate. With such a change, the
apis are cleaner, and the error handling is simplified.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Mime
View raw message