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

Type signatures -- offering a consistent API for easy use #760

Open
typesanitizer opened this issue Jul 8, 2017 · 8 comments
Open

Type signatures -- offering a consistent API for easy use #760

typesanitizer opened this issue Jul 8, 2017 · 8 comments

Comments

@typesanitizer
Copy link
Contributor

I'm primarily talking about the BatVect module below, but perhaps this point applies to other modules too (I haven't used them much).

Having the data structure be the final argument of the function is really useful when using a chain of pipe operators; say you're working with a vector of vectors:

let x = [some computation]
        |> (fun v -> V.modify v p1 (fun z -> V.set z p2 y) 

versus a hypothetical

let x = [some computation]
        |> V.new_modify p1 (V.new_set p2 y) 

Obviously one can define such functions locally by rearranging the arguments, but I think that the library definitions should offer a consistent API where the last argument is always[1] the data structure for readability of long chains of pipe operations such as this.

Some functions in BatVect follow this, such as append, concat etc. but there are many which do not such as modify, set and so on.

For example, Haskell's Data.List follows this guideline and this point is explicitly mentioned in Elm's package design guidelines.

[1] For the specific case of fold-like operations, I'm not arguing one way or another (for the inner map). In this situation, Haskell and Elm are different. Haskell has

foldl :: (b -> a -> b) -> b -> t a -> b 
foldr :: (a -> b -> b) -> b -> t a -> b

whereas Elm's foldr and foldl signatures are identical.

foldl : (a -> b -> b) -> b -> List a -> b
foldr : (a -> b -> b) -> b -> List a -> b

However, these functions still have the list as the last argument. I can understand that you might want to have the order of arguments be consistent with Pervasives and so you have the foldable type as the penultimate argument for foldr.

@UnixJunkie
Copy link
Member

The ocaml solution is to use labeled arguments. Look at BatList.Labels for example.
Then you can use the labeled parameters in whatever order you like.
I agree with you that "Having the data structure be the final argument of the function" is nice,
but it's a little bit late to enforce such a convention to the whole batteries library.

@typesanitizer
Copy link
Contributor Author

Look at BatList.Labels for example.

Thanks for the suggestion; it is a good alternative. However, BatVect (and many other modules) do not have such a module; are there any plans for adding them/would you accept a PR for it?

but it's a little bit late to enforce such a convention to the whole batteries library.

Are there any plans to have a future release with a bunch of breaking changes (to include new features)? Perhaps one could add this to the list.

@UnixJunkie
Copy link
Member

I am personally not in favour of having more labelled modules.
I think it is just some code duplication and I personally never use those modules.
They also increase the maintenance cost of existing code.
However, I am just one of the several maintainers.
If you want to contribute such code, it might be accepted depending on what the other maintainers think about it.

Concerning your other point, the next major release is supposed to have breaking changes.
You can label such changes in the bug tracker.
If you are planning to enforce the convention "Having the data structure be the final argument of the function"
for all batteries module, this needs to be accepted by most maintainers and users of the library.
That would need a lot of not so interesting work on many files in batteries.

@gasche
Copy link
Member

gasche commented Jul 14, 2017

I am personally fine with having Labels submodule. It is true that they generate some maintenance burden, because each function has to be duplicated there and name changes propagated etc., but if they are contributed by users that care about them I think it's worth the cost.

On the other hand, I don't really believe in the idea of changing Batteries 3 to use a completely different API convention that the current release. Batteries is, in part, defined by the fact that it strives to remain a drop-in extension of the standard library, instead of an opinionated library that defines its own ways to do things are requires you to change your code. If you want library with more innovative designs, there are Jane Street Core (and now Base) and Simon Cruanes' Containers that are both interesting choices.

@typesanitizer
Copy link
Contributor Author

typesanitizer commented Jul 14, 2017

Well, the project's readme does mention this near the very top:

provides a consistent API for otherwise independent libraries.

but I understand your points -- changing the API so drastically is perhaps too much work for not much benefit and with high costs (users have to change their code, breaking compatibility with the standard library etc.).

Okay, so changing the API is out of the question now.

@gasche could you ask other maintainers to weigh in on the point about adding Labels submodules? (I'm not sure who all are maintainers.) If a majority say yes (assuming you folks follow a majority based system), I'll go ahead and submit a patch.

Also, does the project have written contribution guidelines? I couldn't find contributing.md or a similar file.

@gasche
Copy link
Member

gasche commented Jul 14, 2017

@thizanne?

@UnixJunkie
Copy link
Member

UnixJunkie commented Jul 16, 2017 via email

typesanitizer pushed a commit to typesanitizer/batteries-included that referenced this issue Jul 17, 2017
* Partial fix for issue ocaml-batteries-team#760.
* Also fixes a few typos in the `BatVect` documentation.
typesanitizer pushed a commit to typesanitizer/batteries-included that referenced this issue Jul 17, 2017
* Partial fix for issue ocaml-batteries-team#760.
* Also fixes a few typos in the `BatVect` documentation.
@thizanne
Copy link
Member

thizanne commented Jul 18, 2017

I agree with @gasche here.

Besides I have an almost common usecase for Labels submodules : when I'm folding over, say, a Set.t with the accumulator also being a Set.t, relying on me knowing the order of the parameters gave me enough annoying bugs.

Concerning the breakage on next major release to get a more consistent interface, I don't have any strong opinion (probably because we have already so many things to break that I don't feel I need to have an opinion on this :) ).

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

No branches or pull requests

4 participants