scala-hamsters / hamsters

A mini Scala utility library
Apache License 2.0
291 stars 23 forks source link

[Issue-35] Added orEmpty and orZero functions for Option #44

Closed sohum2002 closed 7 years ago

sohum2002 commented 7 years ago
dgouyette commented 7 years ago

Thanks for your contribution @sohum2002 . I'll check it ASAP

sohum2002 commented 7 years ago

@dgouyette I've made some changes as per your comments, thanks for that! Regarding Monoid's, I'm not too sure how they are used in Scala, will look into it.

EDIT: I did a quick search on Monoids and don't think it's suitable to implement this for Monoids.

sohum2002 commented 7 years ago

Thank you @loicdescotte for your comments to help improve my pull request. I made the changes you recommended, so when you're free please take a look! Looking forward to it :)

dgouyette commented 7 years ago

@sohum2002 Monoids provide a zero element. So i think , that would help http://eed3si9n.com/learning-scalaz/Monoid.html because orEmpty give us a zero element for a given type

sohum2002 commented 7 years ago

@dgouyette Monoids look like an interesting implementation for hamsters project. However, I think this should be separated out into another issue/feature request? Let me know what you think!

loicdescotte commented 7 years ago

Nice, thanks :)
Can you git rebase (squash) your branch before the merge, so we have a single commit for this ?

sohum2002 commented 7 years ago

@loicdescotte Squashed and ready to merge :)

loicdescotte commented 7 years ago

Thanks!