commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Herbert <alex.d.herb...@gmail.com>
Subject Re: [commons-statistics] branch STATISTICS-14 updated: WIP - [...]
Date Tue, 04 Jun 2019 21:55:12 GMT


> On 4 Jun 2019, at 22:43, Gilles Sadowski <gilleseran@gmail.com> wrote:
> 
> Hi.
> 
> Is there a purpose to having mails from "WIP" commits?
> It's difficult to know when/what to comment about.
> 
> Regards,
> Gilles
> 
> P.S. In the below commits, I don't understand the purpose of
> "UNKNOWN".  Also, I'd say that having a named constant ZERO
> for the value "0" is overkill.

Perhaps WIP should be done on a forked GitHub repo instead. When all complete it can be reviewed
in a PR. You can still ask for feedback by sharing the forked repo URL.

I agree on the code comments.

ZERO is a magic number that is ignored by most code checkers and use of a constant is odd.

The UNKNOWN value is redundant as the min/max is never returned when they are defined as UNKNOWN.
The code currently throws an exception which was discussed on a previous commit thread as
one possible choice, the other was returning null to represent undefined.

> 
> Le mar. 4 juin 2019 à 22:53, <khmarbaise@apache.org> a écrit :
>> 
>> This is an automated email from the ASF dual-hosted git repository.
>> 
>> khmarbaise pushed a commit to branch STATISTICS-14
>> in repository https://gitbox.apache.org/repos/asf/commons-statistics.git
>> 
>> 
>> The following commit(s) were added to refs/heads/STATISTICS-14 by this push:
>>     new c634122  WIP - [STATISTICS-14] - BigDecimalStatistics - Continued.  - Fixed
PMD issues. Only one left:    PMD Failure: ....BigDecimalSummaryStatistics:25 Rule:DataClass
Priority:3    The class 'BigDecimalSummaryStatistics' is suspected to be a Data Class (WOC=25.000%,
NOPA=0, NOAM=4, WMC=27).
>> c634122 is described below
>> 
>> commit c634122fda93aa4bfaae2f3ce5b523a9f8e6b6d4
>> Author: Karl Heinz Marbaise <khmarbaise@apache.org>
>> AuthorDate: Tue Jun 4 22:50:35 2019 +0200
>> 
>>    WIP - [STATISTICS-14] - BigDecimalStatistics - Continued.
>>     - Fixed PMD issues. Only one left:
>>       PMD Failure: ....BigDecimalSummaryStatistics:25 Rule:DataClass Priority:3
>>       The class 'BigDecimalSummaryStatistics' is suspected to be a Data Class (WOC=25.000%,
NOPA=0, NOAM=4, WMC=27).
>> ---
>> .../descriptive/BigDecimalSummaryStatistics.java   | 36 +++++++++++++++-------
>> 1 file changed, 25 insertions(+), 11 deletions(-)
>> 
>> diff --git a/commons-statistics-bigdecimal/src/main/java/org/apache/commons/statistics/bigdecimal/descriptive/BigDecimalSummaryStatistics.java
b/commons-statistics-bigdecimal/src/main/java/org/apache/commons/statistics/bigdecimal/descriptive/BigDecimalSummaryStatistics.java
>> index ad5f05a..8eae837 100644
>> --- a/commons-statistics-bigdecimal/src/main/java/org/apache/commons/statistics/bigdecimal/descriptive/BigDecimalSummaryStatistics.java
>> +++ b/commons-statistics-bigdecimal/src/main/java/org/apache/commons/statistics/bigdecimal/descriptive/BigDecimalSummaryStatistics.java
>> @@ -25,6 +25,10 @@ import java.util.function.Consumer;
>> public class BigDecimalSummaryStatistics implements Consumer<BigDecimal> {
>> 
>>     /**
>> +     * This is used to assign min/max a useful value which is not {@code null}.
>> +     */
>> +    private static final BigDecimal UNKNOWN = BigDecimal.ZERO;
>> +    /**
>>      * The count value for zero.
>>      */
>>     private static final long ZERO_COUNT = 0L;
>> @@ -46,14 +50,21 @@ public class BigDecimalSummaryStatistics implements Consumer<BigDecimal>
{
>>     private BigDecimal max;
>> 
>>     /**
>> +     * This keeps the information if min/max have been assigned a correct value
or not.
>> +     */
>> +    private boolean minMaxAssigned;
>> +
>> +    /**
>>      * Create an instance of BigDecimalSummaryStatistics. {@code count = 0} and sum
= {@link
>>      * BigDecimal#ZERO}
>>      */
>>     public BigDecimalSummaryStatistics() {
>>         this.count = ZERO_COUNT;
>>         this.sum = BigDecimal.ZERO;
>> -        this.max = null;
>> -        this.min = null;
>> +
>> +        this.minMaxAssigned = false;
>> +        this.max = UNKNOWN;
>> +        this.min = UNKNOWN;
>>     }
>> 
>>     /**
>> @@ -102,6 +113,7 @@ public class BigDecimalSummaryStatistics implements Consumer<BigDecimal>
{
>> 
>>             this.min = min;
>>             this.max = max;
>> +            this.minMaxAssigned = true;
>>         }
>> 
>>     }
>> @@ -121,12 +133,13 @@ public class BigDecimalSummaryStatistics implements Consumer<BigDecimal>
{
>>         count++;
>>         sum = sum.add(value);
>> 
>> -        if (min == null) {
>> -            min = value;
>> -            max = value;
>> -        } else {
>> +        if (minMaxAssigned) {
>>             min = min.min(value);
>>             max = max.max(value);
>> +        } else {
>> +            min = value;
>> +            max = value;
>> +            minMaxAssigned = true;
>>         }
>>     }
>> 
>> @@ -144,12 +157,13 @@ public class BigDecimalSummaryStatistics implements Consumer<BigDecimal>
{
>>         count += other.count;
>>         sum = sum.add(other.sum);
>> 
>> -        if (min == null) {
>> -            min = other.min;
>> -            max = other.max;
>> -        } else {
>> +        if (minMaxAssigned) {
>>             min = min.min(other.min);
>>             max = max.max(other.max);
>> +        } else {
>> +            min = other.min;
>> +            max = other.max;
>> +            minMaxAssigned = true;
>>         }
>>     }
>> 
>> @@ -209,7 +223,7 @@ public class BigDecimalSummaryStatistics implements Consumer<BigDecimal>
{
>>      * @return The arithmetic mean of values, or zero if none
>>      */
>>     public final BigDecimal getAverage() {
>> -        if (this.count > 0) {
>> +        if (this.count > ZERO_COUNT) {
>>             return this.sum.divide(BigDecimal.valueOf(this.count));
>>         } else {
>>             return BigDecimal.ZERO;
>> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message