Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coerce #15

Merged
merged 4 commits into from
Nov 27, 2018
Merged

Coerce #15

merged 4 commits into from
Nov 27, 2018

Conversation

buscarini
Copy link
Contributor

I implemented an extension to be able to force cast between compatible Tagged data types where the raw representation is the same but the tag changes. This should allow to switch tags when needed instead of creating new wrappers. I hope you'll find it useful. I'm not sure if this is the best solution in terms of usage, though. Here are some equivalent options:

let x2: Tagged<Tag2, Int> = x.coerce()

let x2 = x.coerce(Tagged<Tag2, Int>.self)

let x2 = x.coerce(Tag2.self)

I went with the first one, but I think a case could be made for the rest. I'm not sure if adding all of them would create ambiguity for the compiler.

@stephencelis
Copy link
Member

This sounds like a good addition! Curious what @mbrandonw thinks since it may clean up this PR a bit.

@mbrandonw
Copy link
Member

I definitely like this operation. The Haskell Tagged library calls this retag, but that sounds maybe too innocuous, as retagging something is a pretty significant semantic change to a value. So I think coerce is a good name to show the seriousness of it.

Also I like the 3rd signature that passes in the tag explicitly. @stephencelis what do you think?

@stephencelis
Copy link
Member

Agree! I like the third signature the best.

@mbrandonw
Copy link
Member

@buscarini wanna make the change to use the 3rd signature and then we can merge it in?

let x: Tagged<Tag, Int> = 1

enum Tag2 {}
let x2: Tagged<Tag2, Int> = x.coerce()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also looks like there is some weird spacing here. we use 2 space tabs.

@buscarini
Copy link
Contributor Author

Ok, I switched to the third version, and I also fixed the spacing issues.

@mbrandonw
Copy link
Member

Sorry to suggest one more change, but could we also update the API to be coerced(to:)? Thinking that follows the swift naming guidelines a lil better.

Once that's done I'll merge!

@buscarini
Copy link
Contributor Author

Agreed 👍

Copy link
Member

@mbrandonw mbrandonw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one last thing, added the to label. if you bring in these suggested changes I'll merge!

thanks again for this!

Sources/Tagged/Tagged.swift Outdated Show resolved Hide resolved
Tests/TaggedTests/TaggedTests.swift Outdated Show resolved Hide resolved
Incorporated fixes for argument prefix

Co-Authored-By: buscarini <buscarini@gmail.com>
@buscarini
Copy link
Contributor Author

Sorry, I missed the to argument.

@mbrandonw
Copy link
Member

No prob, will merge when the build passes!

@mbrandonw mbrandonw merged commit e2b7d5f into pointfreeco:master Nov 27, 2018
stephencelis pushed a commit that referenced this pull request Apr 9, 2019
* Coerce (#15)

* Added coerce to be able to switch between compatible representations where only the tag changes

* Change coerce to explicitly pass the new tag. Fixed spacing issues

* Renamed coerce to coerced

* Apply suggestions from code review

Incorporated fixes for argument prefix

Co-Authored-By: buscarini <buscarini@gmail.com>

* XcodeGen (#17)

* XcodeGen

* 4.2

* Bump sim

* Update package names

* Bump Xcode

* Add workspace (#18)

* Add tests for playgrounds (#20)

* Add tests for playgrounds.

* Update Makefile

Co-Authored-By: mbrandonw <mbw234@gmail.com>

* Time != Money (#10)

* Time

* Money

* typo

* podspecs

* update readme

* comment about currencies

* date conversions and more tests

* positive examples in readme

* clean up

* more cleanup

* Update README.md

* wip

* Update Tagged.podspec

* clean up

* clean u

* Bump

* Update Podspecs

* run make xcodeproj

* dependency

* make xcodeproj

* Bundle XcodeGen. Ignore Package.resolved

* Fix podspecs

* Prep pod release

* Fix pod name

* Swift 5 (#19)

* Add workspace

* Swift 5

* Bumps

* Workaround for Swift bug.

* Bumps

* Bump 2

* Podspecs

* Use Swift 5 Docker

* Conditionally include development dependencies

* Use PF-prefixed environment variable to scope

* Update TaggedTests.swift

* Revert "Update TaggedTests.swift"

This reverts commit b5bea0b.

* Update README.md

* Remove unneeded return

* Update podspecs to point to 0.4.0.

* Fixed date parsing with a custom decoding strategy

* Fixed encoding with a custom date strategy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants