[statistics] - STATISTICS-14

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[statistics] - STATISTICS-14

Karl Heinz Marbaise-3
Hi,

created an appropriate branch[1] with a first implementation to check If
this is going into the right direction if not please let me know...

Kind regards
Karl Heinz Marbaise

[1]:
https://github.com/apache/commons-statistics/commit/4e65221a692dbf8346d2ac9ca9958a6e879b4da5

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [statistics] - STATISTICS-14

Alex Herbert


> On 1 Jun 2019, at 20:59, Karl Heinz Marbaise <[hidden email]> wrote:
>
> Hi,
>
> created an appropriate branch[1] with a first implementation to check If
> this is going into the right direction if not please let me know…
>

I do not see why the private count variable is an object Long and not a primitive long.

Why the explicit BigDecimalConsumer interface? Why not Consumer<BigDecimal>? Use the JDK standard Consumer interface and you get the default methods added for ‘free’ and do not add a redundant interface to the codebase.

In the combine and accept methods you check both min and max for null. But only one null check is required as both are set together (i.e. when count is 0 they should be null, otherwise they will not be).

Following DoubleSummaryStatistics, perhaps min and max should return the appropriate opposite infinity bounds when count is zero, i.e.

Min = Double.POSITIVE_INFINITY
Max = Double.NEGATIVE_INFINITY

Regards,

Alex


> Kind regards
> Karl Heinz Marbaise
>
> [1]:
> https://github.com/apache/commons-statistics/commit/4e65221a692dbf8346d2ac9ca9958a6e879b4da5
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [statistics] - STATISTICS-14

Karl Heinz Marbaise-3
Hi,

On 01.06.19 23:51, Alex Herbert wrote:

>
>
>> On 1 Jun 2019, at 20:59, Karl Heinz Marbaise <[hidden email]> wrote:
>>
>> Hi,
>>
>> created an appropriate branch[1] with a first implementation to check If
>> this is going into the right direction if not please let me know…
>> >
> I do not see why the private count variable is an object Long and not a primitive long. >
> Why the explicit BigDecimalConsumer interface? Why not Consumer<BigDecimal>? Use the JDK standard Consumer interface and you get the default methods added for ‘free’ and do not add a redundant interface to the codebase.

Makes of course more sense...

>
> In the combine and accept methods you check both min and max for null. But only one null check is required as both are set together (i.e. when count is 0 they should be null, otherwise they will not be).
>
> Following DoubleSummaryStatistics, perhaps min and max should return the appropriate opposite infinity bounds when count is zero, i.e.
>
> Min = Double.POSITIVE_INFINITY
> Max = Double.NEGATIVE_INFINITY

Thanks for your comments and hints..
I will change that..


Kind regards
Karl Heinz Marbaise

>
> Regards,
>
> Alex
>
>
>> Kind regards
>> Karl Heinz Marbaise
>>
>> [1]:
>> https://github.com/apache/commons-statistics/commit/4e65221a692dbf8346d2ac9ca9958a6e879b4da5

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [statistics] - STATISTICS-14

Eitan Adler
(please make sure to CC me on replies)

On Sun, 2 Jun 2019 at 01:09, Karl Heinz Marbaise <[hidden email]> wrote:

> Hi,
>
> On 01.06.19 23:51, Alex Herbert wrote:
> >
> >
> >> On 1 Jun 2019, at 20:59, Karl Heinz Marbaise <[hidden email]> wrote:
> >>
> >> Hi,
> >>
> >> created an appropriate branch[1] with a first implementation to check If
> >> this is going into the right direction if not please let me know…
> >> >
> > I do not see why the private count variable is an object Long and not a
> primitive long. >
> > Why the explicit BigDecimalConsumer interface? Why not
> Consumer<BigDecimal>? Use the JDK standard Consumer interface and you get
> the default methods added for ‘free’ and do not add a redundant interface
> to the codebase.
>
> Makes of course more sense......
>

Reviewed on the commit.

--
Eitan Adler