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

Bad return type for [Use Case] docs in Range #5678

Closed
scabug opened this issue Apr 17, 2012 · 5 comments
Closed

Bad return type for [Use Case] docs in Range #5678

scabug opened this issue Apr 17, 2012 · 5 comments
Assignees
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Apr 17, 2012

Many [Use case] entries in the documentation of Range report Range as return type when it should be IndexedSeq (or Seq[...]).

@scabug
Copy link
Author

@scabug scabug commented Apr 17, 2012

Imported From: https://issues.scala-lang.org/browse/SI-5678?orig=1
Reporter: fabien sartor (fasar)
Affected Versions: 2.9.0, 2.9.2

Loading

@scabug
Copy link
Author

@scabug scabug commented Apr 17, 2012

@dcsobral said:
Please give an example. An use case entry gives a simplified type signature that adequately explains one use case, not the general case. The full type signature is available elsewhere, and IndexedSeq or Seq are not correct for them.

Loading

@scabug
Copy link
Author

@scabug scabug commented Apr 18, 2012

fabien sartor (fasar) said:
I use scala 2.9.2:

# scala -version
Scala code runner version 2.9.2 -- Copyright 2002-2011, LAMP/EPFL

In scala API doc at Range page there is an use case of union (http://www.scala-lang.org/api/current/scala/collection/immutable/Range.html)
The use case says :

def union(that: Seq[Int]): Range[A] 

Then when I use union, Range union produces IndexedSeq.

scala> Range(1,5).union(Range(4,8))
res0: scala.collection.immutable.IndexedSeq[Int] = Vector(1, 2, 3, 4, 4, 5, 6, 7)

Range(1,5).union(Seq(4,3,2))
res1: scala.collection.immutable.IndexedSeq[Int] = Vector(1, 2, 3, 4, 4, 3, 2)

The doc explains :

def union[B >: Int, That](that: GenSeq[B])(implicit bf: CanBuildFrom[IndexedSeq[Int], B, That]): That 

But it's more difficult to read and possibly not corroborating with the usecase.

Loading

@scabug
Copy link
Author

@scabug scabug commented Apr 19, 2012

@dcsobral said:
Yes, the real type signature is more difficult to read, which is why there are use cases.

Anyway, I have looked into it, and verified that there's no CanBuildFrom that creates a Range. Furthermore, it seems $Coll (as opposed to $coll) is only used on use cases, so IndexedSeq would be better. Or, perhaps, just do not redefine it on Range (I'm not sure if it gets inherited from IndexedSeq or not).

It's a very small change, but one does have to build the docs to look them over, and I'm a bit out of time these days. Maybe someone will come and pick this up, but this sort of ticket is very low priority. If you had more experience with Scala, I'd advise to look into submitting a pull request for it yourself. And if you have enough git, github and ant experience, you should try it -- just remove the line where $Coll is defined on the source file for Range, plus do all the fork/branch/edit/test/push/pull request work. People on irc and mailing lists can help you with it.

Loading

@scabug
Copy link
Author

@scabug scabug commented Dec 10, 2012

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants