-
Notifications
You must be signed in to change notification settings - Fork 30
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
Rename the parameters of Range.exponentialFrom (and friends) #313
Comments
I think this is an improvement |
I thought |
I don't think Mark's argument applies here. Here is his first point including what I think is an implicit assumption.
These three values all have the same type, but they play different roles. I think the following is ambiguous given only the API. Suppose I want to represent an exponential range (of years) from 1900 to 2100 that shrinks towards 2000. How should I write this?
I only know the correct order because I looked at the implementation. The correct order is the last one. The names suggested by @dharmaturtle would remove this ambiguity. For an example, consider this line I added in PR #239. let addChild (child: Tree<'a>) (parent: Tree<'a>) : Tree<'a> =
let (Node (x, xs)) = parent
Node (x, Seq.cons child xs) Since the both arguments have the same type, I think it is important to use the identifiers to distinguish how they are used. For another example, consider |
I agree with your point of view. I believe that this point of view is basically also the point of view of most of the users out there (that don't know―or don't want to know―the history and/or the implementation behind all this). It can be seen as an improvement, then, 👍 |
I agree with Mark that single-letting names are often perfectly clear. I am just not convinced that this is one of them. Maybe one way to improve the situation is introducing another type. Maybe call it The issue of functions with arguments of the same type is interesting. It is often said that functions with cyclomatic complexity |
fsharp-hedgehog/src/Hedgehog/Range.fs
Lines 172 to 174 in 3684694
z
,x
, andy
are pretty opaque. How do we feel about renaming these toorigin
,lowerBound
, andupperBound
respectively?I intend the rename to also affect
exponentialBounded
,linear*
, andconstant*
.Haskell uses
z
,x
, andy
, but it looks like they have docs on each specific parameter... which F# can't do yet 🙄Thoughts?
The text was updated successfully, but these errors were encountered: