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

Make the DList type abstract #4

Closed
spl opened this issue Nov 13, 2013 · 11 comments · May be fixed by basvandijk/dstring#1
Closed

Make the DList type abstract #4

spl opened this issue Nov 13, 2013 · 11 comments · May be fixed by basvandijk/dstring#1
Assignees
Milestone

Comments

@spl
Copy link
Owner

spl commented Nov 13, 2013

DList a is isomorphic to [a] via fromList/toList, but [a] -> [a] is not isomorphic to [a]. That is, all lists are dlists, and all dlists are lists only if there is no way to construct a y :: DList a from an "unsafe" x :: [a] -> [a].

Currently, the library exports the constructor DL and deconstructor unDL which allows the construction of unsafe dlist values. Consider this GHCi session:

*Data.DList> let x = DL $ \z -> z ++ z
*Data.DList> toList x
[]
*Data.DList> fromList $ toList x
Data.DList.fromList []

The values of x and toList x are clearly not isomorphic. When used:

*Data.DList> append x (singleton 'a')
Data.DList.fromList "aa"
*Data.DList> append (fromList $ toList x) (singleton 'a')
Data.DList.fromList "a"

DList was abstract early on. I don't know why it was changed.

To preserve the isomorphism, I think DList should be an abstract type and the constructor and deconstructor should be removed from the export list.

@spl
Copy link
Owner Author

spl commented Nov 13, 2013

I did a quick search of Hackage to see which packages are using DL/unDL. The only one I came up with was dstring.

@basvandijk What do you think about this? Your fromShowS and toShowS share the same problem as described above.

@basvandijk
Copy link
Contributor

Good point. Lets make DList abstract.

However I think we can keep the export of unDL. You can't use it to create unsafe DLists.

With regards to dstring I think it's sufficient to just remove the fromShowS function.

@spl
Copy link
Owner Author

spl commented Nov 14, 2013

True, only the constructor is unsafe, not the deconstructor, so we can still use unDL. However, I think a more descriptive name would be suitable. How about one of the following?

apply          :: DList a -> [a] -> [a]
undlist
toList'
toListFunction
toListFun
toFunction
fromDList

I think I prefer apply (because that is what the function intuitively does by being a replacement for $ in toList) and toFunction (because you can think of it as a conversion from DList a to [a] -> [a]).

@basvandijk
Copy link
Contributor

I like apply.

@ghost ghost assigned spl Nov 14, 2013
@spl spl closed this as completed in 27e499e Nov 18, 2013
@dag
Copy link

dag commented Nov 19, 2013

You actually can use unDL to construct any DList:

x = empty { unDL = \z -> z ++ z }

The same is not true for apply because record selectors aren't first class. Thus, whenever unDL is removed, this should not be done by renaming it to apply.

@spl
Copy link
Owner Author

spl commented Nov 19, 2013

Excellent point, @dag. Thanks!

mamash pushed a commit to TritonDataCenter/pkgsrc-wip that referenced this issue Nov 29, 2013
Change Log
==========

Version 0.6 (2013-11-29) *Black Friday*
---------------------------------------

#### Development changes

* Maintenance and development taken over by Sean Leather
* Migrate repository from http://code.haskell.org/~dons/code/dlist/ to
  https://github.com/spl/dlist

#### Package changes

* Stop supporting `base < 2`
* Fix tests and use `cabal test`
* Add scripts for running `hpc`
* Update documentation

#### New features

* New type class instances: `Eq`, `Ord`, `Read`, `Show`, `Alternative`,
  and `Foldable`
* New function `apply` to use instead of `unDL`

#### Deprecations

* Deprecate DList constructor and record selector to make it abstract
  (see [#4](spl/dlist#4))
* Deprecate `maybeReturn` which is not directly relevant to dlists
@jimstutt
Copy link

I found unDL in planar-graph and substituted apply. What is the preferred syntax? Tnx.

@spl
Copy link
Owner Author

spl commented Apr 23, 2014

@jimstutt unDL is no longer available as a record selector (a.k.a. field). It can no longer be used in a “setter” way, e.g. DL { unDL = ... }. This is not allowed because constructing DLists in this way goes around the expected isomorphism. But apply can replace unDL in other situations. Does that help?

@jimstutt
Copy link

It was @dag's comment "Thus, whenever unDL is removed, this should not be done by renaming it to apply." which prompted me to ask. I'll just use apply.

@dag
Copy link

dag commented Oct 1, 2014

@jimstutt Sorry for the late response here, but anyway: what I meant was that when unDL is removed from the dlist package itself it's not enough to simply rename the record field and export the new name, because record fields can be used as setters, and can be used to break module boundaries otherwise imposed by private constructors. My remark was not meant to say anything about code using the dlist package. Hope that clears it up!

@jimstutt
Copy link

jimstutt commented Oct 2, 2014

Thanks dag. I'll have to revisit the code.

FranklinChen added a commit to FranklinChen/dstring that referenced this issue May 5, 2019
Use recommendation in https://prime.haskell.org/wiki/Libraries/Proposals/SemigroupMonoid to support different GHC versions for Semigroup and Monoid.
Remove SAFE.
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 a pull request may close this issue.

4 participants