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

[Archie Cobbs]

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/830cb213/attachment-0001.htm>