search

dpaukov / combinatoricslib3

Combinatorial objects stream generators for Java.
Apache License 2.0
177 stars 24 forks source link

Support KPermutation #10

Closed fengertao closed 3 years ago

coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.008%) to 99.744% when pulling f1eca992099653ec3fc7610b10600aaf528812be on fengertao:develop into dfc2c23062ea45881b81aee1ae5a3fde924386b4 on dpaukov:master.

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-0.2%) to 99.513% when pulling 6ea8210e930b264ca7a428178c7285e9e1888be9 on fengertao:develop into dfc2c23062ea45881b81aee1ae5a3fde924386b4 on dpaukov:master.

fengertao commented 3 years ago

Hi, Paukov:

Any comment to the changed PR?

dpaukov commented 3 years ago

Hello Charlie,

I am sorry for missing the comments. It's been very busy month on my side.

I agree with your idea to use a flag to let users decide whether they need to support duplicates or not. However I think the generator should produce all the k-permutations (without calling distinct()) by default (if no flags are provided) even if an input vector contains duplicates. Also I believe we can reuse the existing enum TreatDuplicatesAs without introducing a new one that can confuse users. What do you think if we merge this PR and I will add another PR that removes your enum kTreatDuplicatesAs and changes the default implementation?

fengertao commented 3 years ago

Agree with you the default behavior is allow duplicate, and some optional flag can remove duplicate. Please merge and change default implementation as you purosed.