On Fri, 26 Jun 2026 17:38:10 GMT, Nir Lisker <[email protected]> wrote:
>> my version is better as it avoids scanning twice > > This claim might be too strong. From a performance perspective, the sizes of > the collections are very small (a `DataFormat` will realistically have at > most 3 ids and the total number of ids/mime-types that exist in the wild is > probably in the dozens only), so performance is not really a consideration. > However, if we do look at it, your implementation iterates over the array, > which is O(n), with a hashed `get` which is O(1). `Collections.disjoint` is > optimized to the given collections - it iterates over the smaller collection, > which is O(n), and checks contains which is O(1). So I don't think your code > gives better performance. > > From a readability perspective, what your code says is "for each id, find a > data format that uses it" (and if the `DataFormat` shares 2 or more ids, you > will find and compare the same `DataFormat` twice or more). `disjoint` is > clearer: "does any of the new ids exist already?" > > In fact, I'm not sure why you're throwing only when not all the identifiers > are the same, that is, why you specified "if one of the given ids is already > assigned to another DataFormat **with a different set of ids**". If some are > the same it's an exception, but if all of them are the same then it's not? > Maybe I'm missing something. my version is faster, but the difference is so minute it's irrelevant. I can use `disjoint()` here. > If some are the same it's an exception, but if all of them are the same then > it's not? This is the essence of the change. We are to allow multiple DataFormat instances if they are equal, so as to avoid the unnecessary exceptions. The reason we can't allow disjoint formats is because the `lookupMimeType()` logic will break, and that would be a more drastic breaking change. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2197#discussion_r3483791318
