Re: [commons-statistics] branch STATISTICS-14 updated: WIP - [...]
> On 4 Jun 2019, at 22:43, Gilles Sadowski 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, 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:3The 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
>> 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 {
>>
>> /**
>> + * 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 {
>> 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 {
>>
>> this.min = min;
>> this.max = max;
>> +this.minMaxAssigned = true;
>> }
>>
>> }
>> @@ -121,12 +133,13 @@ public class BigDecimalSummaryStatistics implements
>> Consumer {
>> 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 {
>> count += other.count;
>> sum = sum.add(other.sum);
>>
>> -if (min == null) {
>> -min = other.min;
>> -max = other.max;
>>
Re: [commons-statistics] branch STATISTICS-14 updated: WIP - [...]
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.
Le mar. 4 juin 2019 à 22:53, 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:3The 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
> 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 {
>
> /**
> + * 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 {
> 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 {
>
> this.min = min;
> this.max = max;
> +this.minMaxAssigned = true;
> }
>
> }
> @@ -121,12 +133,13 @@ public class BigDecimalSummaryStatistics implements
> Consumer {
> 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 {
> 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 {
> * @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;
>
