jcabi / jcabi-immutable

Primitive Java Immutable Collections, like Array, ArraySet, etc.
https://immutable.jcabi.com
Other
31 stars 18 forks source link

Wrong use of generics in ArrayComparator#Reverse #28

Closed piddubnyi closed 8 years ago

piddubnyi commented 8 years ago

Solving https://github.com/jcabi/jcabi-immutable/issues/21 Changed generic usage to avoid wrong usage.

dmarkov commented 8 years ago

@piddubnyi I will find a reviewer for your pull requests shortly, thanks for contribution!

dmarkov commented 8 years ago

@pinaf could you please review this PR

pinaf commented 8 years ago

@piddubnyi let's add the proposed test case.

pinaf commented 8 years ago

@piddubnyi <T extends Comparable<T> seems recursive to me. perhaps you mean <T extends Comparable>?

piddubnyi commented 8 years ago

@pinaf <T extends Comparable<T>> is kind of recursive, but otherwise qulice complain about raw type, should I add annotation to ignore this check?

Also, the proposed test is just showing the possibility of getting ClassCastException, with new implementation it is just not compiles because of wrong generic types. Only positive test is possible, should I add it ?

pinaf commented 8 years ago

@piddubnyi you are correct, please do add the positive test. We'll see what @yegor256 says about the recursive generics use.

piddubnyi commented 8 years ago

@pinaf Please check if such test is enough.

pinaf commented 8 years ago

@piddubnyi yes it is. please see a few more comments.

piddubnyi commented 8 years ago

@pinaf Fixed

pinaf commented 8 years ago

@rultor merge

rultor commented 8 years ago

@rultor merge

@pinaf Thanks for your request. @yegor256 Please confirm this.

yegor256 commented 8 years ago

@rultor merge

rultor commented 8 years ago

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

rultor commented 8 years ago

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 9min)

dmarkov commented 8 years ago

@pinaf 26 mins was added to your account, many thanks for your contribution (payment AP-50N657616J286232A)! The task took 92 hours and 26 mins.. you're getting extra minutes here (c=11). +26 to your rating, your total score is +6290

dmarkov commented 8 years ago

@rultor deploy now pls

rultor commented 8 years ago

@rultor deploy now pls

@dmarkov OK, I'll try to deploy now. You can check the progress here

rultor commented 8 years ago

@rultor deploy now pls

@dmarkov Done! FYI, the full log is here (took me 6min)