search

guillermomuntaner / Burritos

A collection of Swift Property Wrappers (formerly "Property Delegates")
MIT License
1.33k stars 43 forks source link

DefaultValue Wrapper #4

Closed own2pwn closed 5 years ago

own2pwn commented 5 years ago

Sry, I ain't good at English so I haven't commented the code. Hope the code speaks for itself!

guillermomuntaner commented 5 years ago

Hello @own2pwn

The repo is under MIT and I don't want licensing on source files, hence the Copyright © 2019 Wicked. All rights reserved. line must be removed. I took the liberty to do so, but would like you to accept those changes before merging. I kept the authorship to you.

I also added documentation and tweaked a bit the tests. The wrapper itself is untouched; I just extended the constructor with an extra optional initial value for a bit extra flexibility & added an explicit reset() method.

Let me know what you think; if you are ok with the changes I will merge.

Thanks for contributing. Regards, Guillermo.

own2pwn commented 5 years ago

Sure thing! Probably the copyright was added due to organization was set in Xcode. I don't mind it to be removed :)

own2pwn commented 5 years ago

It would be even better if someone could figure out, if we can have var value: Value instead of the var value: Value!

guillermomuntaner commented 5 years ago

We can remove the implicit unwrapped optional, but then it would become impossible to assign nil to reset the value. This will require using $x.reset(). This would be more “correct” and powerful but probably les usable.

On Sat 29. Jun 2019 at 03:50, own2pwn notifications@github.com wrote:

It would be even better if someone could figure out, if we can have var value: Value! not as an implicitly unwrapped but as just Value instead!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/guillermomuntaner/Burritos/pull/4?email_source=notifications&email_token=ABY7K3TNNM6TTR3L6AKG37LP425VJA5CNFSM4H4IKMBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY3PDYY#issuecomment-506917347, or mute the thread https://github.com/notifications/unsubscribe-auth/ABY7K3VREYIC2QFBERPGFQTP425VJANCNFSM4H4IKMBA .

own2pwn commented 5 years ago

Got you. Then we can leave it as it is and wait if users would have issues with current implementation. So we have reason to change architecture of this wrapper. What do you think?