search

immutables / immutables-vavr

Immutables encodings for Vavr
Other
33 stars 7 forks source link

Issue13 builder add varargs #14

Closed tmtron closed 7 years ago

tmtron commented 7 years ago

Builder.add with VarArgs #13

tmtron commented 7 years ago

Please don't merge this pull request yet. I just realized that there are some problems which were not covered by the test-cases: The add(varargs) methods use a HashSet internally, and thus will ignore duplicates and change the order, which is not okay for sequential types: e.g. Array, etc I'll fix that soon.

io7m commented 7 years ago

'Lo.

Nice work. One small thing: I think the LinkedHashSet change is wrong: It's still using a plain HashSet to receive the varargs, unless I'm mistaken.

io7m commented 7 years ago

Looks good to me. Thanks again!

tmtron commented 7 years ago

Did you forget to merge the pull-request when closing?
I cannot see these changes in the develop branch yet: e.g. VavrArrayEncoding.java does not have addVarArgs

io7m commented 7 years ago

Strange, I'm pretty sure I merged it. I'll re-open and try merging again (merging an already-merged change should either result in an error or a no-op).

io7m commented 7 years ago

That's better.

tmtron commented 7 years ago

Great - thanks.