[External] : Re: Introduce constructor for PriorityQueue with existing collection and custom comparator

[Valeh Hajiyev]

One more request – Current PR cannot be merged without CRS. Could someone
please help me to create a CSR request for the issue below?

https://bugs.openjdk.org/browse/JDK-6356745

Unfortunately, I don't have permission to create a CSR request.

-Valeh

On Thu, Dec 14, 2023 at 9:40 PM Archie Cobbs <archie.cobbs at gmail.com> wrote:

> Looks great - thanks (I'm not an official reviewer so I can't approve it
> though).
>
> -Archie
>
> On Thu, Dec 14, 2023 at 2:37 PM Valeh Hajiyev <valeh.hajiyev at gmail.com>
> wrote:
>
>> Yes, there's no such precondition. Thanks for having a look, I updated
>> the javadoc as you suggested, and linked it to the old existing ticket.
>> It's now ready for review.
>>
>> I would appreciate if you could have a look again.
>>
>> Cheers,
>> Valeh
>>
>> On Thu, Dec 14, 2023 at 4:22 PM Archie Cobbs <archie.cobbs at gmail.com>
>> wrote:
>>
>>> On Thu, Dec 14, 2023 at 4:56 AM Viktor Klang <viktor.klang at oracle.com>
>>> wrote:
>>>
>>>> I presume that the precondition to have the original collection be
>>>> pre-ordered according to the supplied Comparator can be verified by
>>>> checking before adding each element in the collection to the PQ that it
>>>> compareTo equal-or-greater to the previous one?
>>>>
>>>
>>> Hmm...  something is not adding up.
>>>
>>> Either (a) there is a precondition that the collection already be sorted
>>> or (b) there's no such precondition.
>>>
>>> In case (a):
>>>
>>>    - Why isn't the new constructor invoking
>>>    initElementsFromCollection() instead of initFromCollection()?
>>>    - The precondition is not very obvious from the Javadoc description,
>>>    and moreover what happens when the precondition is not met is not
>>>    documented at all.
>>>
>>> In case (b):
>>>
>>>    - The Javadoc is misleading because it (ambiguously) implies there
>>>    is a precondition with the wording "collection that orders its elements
>>>    according to the specified comparator" (the referent of "that" is ambiguous
>>>    - does it refer to the collection or the PriorityQueue?)
>>>
>>> From the PR description it seems clear that there is no such
>>> precondition. So maybe the Javadoc should say this instead:
>>>
>>> Creates a {@code PriorityQueue} containing the elements in the specified
>>>> collection. The {@code PriorityQueue} will order its elements according to
>>>> the specified comparator.
>>>>
>>>
>>> -Archie
>>>
>>> --
>>> Archie L. Cobbs
>>>
>>
>
> --
> Archie L. Cobbs
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/core-libs-dev/attachments/20231214/9c7b5b48/attachment-0001.htm>