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

First approach to NonEmptyList docs #1971

Closed
wants to merge 3 commits into from

Conversation

AlejandroME
Copy link
Contributor

This is the first draft for NonEmptyList docs, as required by #1801.

I'm requesting feedback about this. What should we have to explore in this docs? I've seen the API of NonEmptyList and I find a lot of similarities with plain old List that I don't find useful to address here.

I appreciate your comments about this WIP and suggestions about what should we put in here.

Thanks!

@codecov-io
Copy link

codecov-io commented Oct 15, 2017

Codecov Report

Merging #1971 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1971   +/-   ##
=======================================
  Coverage   95.08%   95.08%           
=======================================
  Files         301      301           
  Lines        4943     4943           
  Branches      125      124    -1     
=======================================
  Hits         4700     4700           
  Misses        243      243

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77ac6bf...2b483c7. Read the comment docs.

@LukaJCB LukaJCB mentioned this pull request Oct 17, 2017
70 tasks
@AlejandroME
Copy link
Contributor Author

@kailuowang and @LukaJCB: can you give me some advice on this? :)

@LukaJCB
Copy link
Member

LukaJCB commented Oct 27, 2017

Hey @AlejandroME Sorry, for not getting back to you earlier, I'm going to give you a review later today :)

Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

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

Thanks! I think this is a solid start.

---
# NonEmptyList

If you have had the opportunity of taking a look to [Validated](validated.html) or [IOR](ior.html), you'll find that one of the most common case is to use `NonEmptyList` with one of these data structures.
Copy link
Member

Choose a reason for hiding this comment

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

s/one of the most/a/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. I'll fix all the typos that you've found.


If you have had the opportunity of taking a look to [Validated](validated.html) or [IOR](ior.html), you'll find that one of the most common case is to use `NonEmptyList` with one of these data structures.

Why? Because it fits nicely in the error reporting cases. As its stated by its name, `NonEmptyList` is a _specialized_ data type that will have, at least, one element. You'll find its behaviour like a list, with the aforementioned constraint. Think of it as a `List` wrapper.
Copy link
Member

Choose a reason for hiding this comment

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

s/As its stated/As stated/

}
```

The `head` of the will be _non empty_, meanwhile, the `tail` can have zero or more elements.
Copy link
Member

Choose a reason for hiding this comment

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

s/non empty, meanwhile/non-empty. Meanwhile/

list.head
```

You've seen the creation of a `NonEmptyList` with one `String` element (_"Hello"_), using `.of` combinator, but also you can create a `NonEmptyList` from a regular `List` like this:
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't call .of a combinator.

It would be nice to show that .of takes a variadic tail, too: NonEmptyList.of(1, 2, 3)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perfect! I'll expand this section

val nel = NonEmptyList.fromList(regularList)
```

But, what happen if you try to convert an empty `List` into a `NonEmptyList`? Let's see:
Copy link
Member

Choose a reason for hiding this comment

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

s/happen/happens/


## All is about safety

`List` has a lot of unsafe methods because it doesn't enforce any constraint (other than its type). If you do this:
Copy link
Member

Choose a reason for hiding this comment

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

Are we assuming that people know what "unsafe" means?

Copy link
Contributor Author

@AlejandroME AlejandroME Oct 27, 2017

Choose a reason for hiding this comment

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

Good point! I could use an example for illustrating the concept.


You can see that you get a `NoSuchElementException` because you're trying to get the head of an empty `List`. The same behaviour applies with methods like `.last` or `.tail`.

Using `NonEmptyList` you'll have the type safety in compile-time for this data type.
Copy link
Member

Choose a reason for hiding this comment

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

I think I might talk more about totality than type safety. Neither List[Orange].head nor NonEmptyList[Orange].head will ever give you an apple. It's just that the NEL will always give you an Orange.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you explain me in more depth? I don't understand you when you say "totality".

Copy link
Member

Choose a reason for hiding this comment

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

Total functions are functions that return a value for every input given. Partial functions (like List#head) don't have that guarantee and aren't defined for certain inputs.

Copy link
Contributor Author

@AlejandroME AlejandroME Oct 27, 2017

Choose a reason for hiding this comment

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

Ah, ok, got it! So, based on @rossabaker feedback, should I rephrase this section to address totality instead of type safety or should I leave this as it is and expand it with totality?

Copy link
Member

Choose a reason for hiding this comment

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

I think totality instead of type safety.

- Fixed some typos.
- Restructured Safety concept ("Safety" instead of "Type Safety").
- Explained briefly "Totality".
@AlejandroME
Copy link
Contributor Author

This is the second iteration of the documentation based on @rossabaker and @LukaJCB feedback.
Apart from the suggested improvements, I don't know what else to address in these docs, so, if there's a lack of some concept important to detail here, please let me know.

Thanks! :)


If you have had the opportunity of taking a look to [Validated](validated.html) or [IOR](ior.html), you'll find that a common case is to use `NonEmptyList` with one of these data structures.

Why? Because it fits nicely in the error reporting cases. As stated by its name, `NonEmptyList` is a _specialized_ data type that will have, at least, one element. You'll find that its behavior is like a list, with the aforementioned constraint. Think of it as a `List` wrapper.
Copy link
Contributor

Choose a reason for hiding this comment

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

What does specialized mean here? When I first read this, I thought about @specialized but I guess it's not? I think this might be confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I mean that is a specific-purpose data type (compared to a regular List). I'll rephrase this.

```tut:book
import cats.data.NonEmptyList

val list = NonEmptyList.of("Hello")
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you thought about adding types to your examples? I think this would demonstrate the difference between of and fromList more clearly:

val list: NonEmptyList[String] = NonEmptyList.of("Hello")

val nel: Option[NonEmptyList[String]] = NonEmptyList.fromList(regularList)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, I'll add it.

@rethab
Copy link
Contributor

rethab commented Nov 14, 2017

As you said in the intro of this PR, NonEmptyList supports many operations that a conventional List does. Perhaps add a comment about that.

Also, you could add an example about cats.syntax.list. I like groupByNel in particular, because for me, the regular groupBy was a typical case where I knew it would only produce non-empty lists, but the types did not express that.

More ideas:

  • reference/relate NonEmptyTraverse
  • reference/relate NonEmptyVector

@AlejandroME
Copy link
Contributor Author

Thank you @rethab for your review. I'll work on your suggestions and refine a little bit this docs.

@kailuowang
Copy link
Contributor

completed through #2107

@kailuowang kailuowang closed this Dec 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants