Validated beginners doc #1903

AlejandroME · 2017-09-09T23:17:48Z

This is an approach to address #1678.

Being my first PR, I pushed to my fork the first draft of this a week ago and @zainab-ali gladly helped me with a revision.
This PR contains fixes to the suggestions made in the revision, grammar fixes and the addition of toValidated/toValidatedNel combinators in the example.

I'm aware of the discussion made in the issue about the detail level of this doc. I tried to document this as clearly as possible without delving into too much detail.

This commit is the first approach to address typelevel#1678, developing the form-validation example.

This commit contains a simple form-validation example and a couple of approaches making use of `Validated`. It aims to solve typelevel#1678

codecov-io · 2017-09-09T23:57:02Z

Codecov Report

Merging #1903 into master will increase coverage by 0.33%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1903      +/-   ##
==========================================
+ Coverage    95.2%   95.54%   +0.33%     
==========================================
  Files         248      248              
  Lines        4379     4420      +41     
  Branches      118      124       +6     
==========================================
+ Hits         4169     4223      +54     
+ Misses        210      197      -13

Impacted Files	Coverage Δ
...rc/main/scala/cats/laws/discipline/Arbitrary.scala	`92.06% <0%> (ø)`	⬆️
core/src/main/scala/cats/data/NonEmptyList.scala	`100% <0%> (ø)`	⬆️
.../src/main/scala/cats/data/ReaderWriterStateT.scala
core/src/main/scala/cats/data/StateT.scala
core/src/main/scala/cats/data/IndexedStateT.scala	`100% <0%> (ø)`
...in/scala/cats/data/IndexedReaderWriterStateT.scala	`100% <0%> (ø)`
core/src/main/scala/cats/data/package.scala	`87.5% <0%> (+1.78%)`	⬆️
core/src/main/scala/cats/syntax/monoid.scala	`100% <0%> (+100%)`	⬆️

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 75bc24a...8172251. Read the comment docs.

LukaJCB · 2017-09-10T07:51:59Z

Hi @AlejandroME, this looks really good and thank you for your contribution!
However, we want our code to be checked by the scala compiler and use tut for that, can you change your example code to use tut? :)

AlejandroME · 2017-09-10T12:20:51Z

@LukaJCB Thank you for your feedback! :)
Sure, I'll take a look at tut and then I'll reach you out again.

- The code examples for form validation are now using `tut` with its respective output in the generated .html. - Fixes some structural issues (`Either` return type in the `Validated` example, use of `mapN` instead of `|@|`). - Adds a deprecation notice about cartesian builder and changes this for using `.mapN`.

AlejandroME · 2017-09-10T16:04:11Z

@LukaJCB It's done! :)

I have a couple of points to state:

In the example of Validated with a for-comprehension, I had to leave it annotated with scala because tut:silent:fail doesn't work (it is throwing an error with the imports expecting that they'll also fail). I found this and I see that is the correct behavior of tut. Also, I left the signatures of Validated, Valid and Invalid annotated with scala for preventing collisions with the rest of the code.
For my dummy tests of the example, I was using 0.9.0 and I've found that in 1.0.0-MF, |@| is deprecated. So I've changed the example to use the suggested .mapN syntax and I left a deprecation notice stating this. As I see, these docs reflect the global behavior of the library. Because of this, I added the warning.
If you find that this should not be here please let me know and I'll fix this.

Thank you!

LukaJCB · 2017-09-10T16:32:38Z

docs/src/main/tut/datatypes/validated.md

+  private def validateAge(age: Int): ValidationResult[Int] =
+    if (age >= 18 && age <= 75) age.validNel else AgeIsInvalid.invalidNel
+
+  def validateForm(username: String, password: String, firstName: String, lastName: String, age: Int): ValidationResult[RegistrationData] = {


Can you perhaps put all the validateN functions into another snippet that's checked by tut, and then only have the validateForm go without tut? I don't think you need the surrounding trait btw :)

Meant to put this on the snippet above 😅

Got it! Working on it and also on the support of <2.11.x Either for-comprehensions (Travis complained about flatMap on this versions).

Thank you for your patience with this! :)

Just import cats.syntax.either._ to get that working :)

Thank you, for you help with the docs!

- The `Validated` approach with the for-comprehension has been splitted in two: the instructions that compile and the for-comprehension that doesn't compile. For the first ones, I've used `tut:silent` and for the second one I've used `tut:book:fail`. With this, all the code of this proposal is checked by tut correctly. - Adds an import for Either to bring .flatMap in Scala versions prior to 2.12.x.

AlejandroME · 2017-09-10T21:57:57Z

@LukaJCB I've fixed the checking with tut as you've suggested: I've split into two parts that example: the validateN, checked as tut:silent and the for-comprehension part, checked as tut:book:fail for getting the output of the compilation.

I'm waiting for the Travis build outcome and further suggestions :)

LukaJCB

LGTM to me overall, with a couple of minor issues :)

LukaJCB · 2017-09-11T08:16:28Z

docs/src/main/tut/datatypes/validated.md

+
+```scala
+sealed abstract class Validated[+E, +A] extends Product with Serializable {
+	// Implementation elided


Tiny nitpick, but can we reduce the indentation here?

Is there a standard used by cats? Spaces, tabs? I'm asking this because all the code snippets here are using tabs, so, if there's a defined standard I can fix the indentation everywhere :)

two spaces is standard :)

Got it! I'll fix it in all the code snippets.

LukaJCB · 2017-09-11T08:19:15Z

docs/src/main/tut/datatypes/validated.md

+  type ValidationResult[A] = Validated[NonEmptyList[DomainValidation], A]
+
+  private def validateUserName(userName: String): ValidationResult[String] =
+    if (userName.matches("^[a-zA-Z0-9]+$")) userName.validNel else UsernameHasSpecialCharacters.invalidNel


Maybe we could give a very quick heads-up on the use of .validNel and .invalidNel? :)

Nevermind, I see you're doing it afterwards 😄

LukaJCB · 2017-09-11T08:31:37Z

docs/src/main/tut/datatypes/validated.md

+
+### Meeting applicative
+
+We have to look into another direction: a for-comprehension plays well in a fail-fast scenario, but the structure in our previous example was designed to catch one error at a time, so, our next step is to tweak the implementation a bit.


Maybe we should mention somewhere here, that in order to get the Applicative instance for Validated the left side needs to have a Semigroup instance :)

Working on it! :)

LukaJCB · 2017-09-11T08:35:00Z

docs/src/main/tut/datatypes/validated.md

+To do this, you'll need to use either `.toValidated` or `.toValidatedNel`. Let's see an example:
+
+```tut:book
+FormValidator.validateForm(


I'm not sure how good this example really is, the fail-fast behaviour of Either is already done at this point, so converting to Validated does not somehow enable the Error accumulation we might want, as you probably know. :)
Maybe we could include an example that doesn't drop the error accumulation?

I was thinking the same. I've done this little example for making use of .invalid/.invalidNel but nothing, apart from this snippet came to my mind. I'll think about this and I'll reach you out again :)

- Fixes indentation in all the snippets (two spaces). - Added a new section ("short detour") in where I explain the `Semigroup` role in the accumulation, with a little portion of `Validated` code. - Adds "Back and forth" section explaining how to convert between `Validated` and `Either`. I can't figure it out how to convert from `Either` to `Validated` 'failing slowly'.

AlejandroME · 2017-09-12T02:01:23Z

@LukaJCB Some notes here:

I've Fixed indentation in all the snippets, as suggested :)
I've added a new section (Short detour) in where I explain the role of a Semigroup in the accumulation and the possibility of use of other structures different to NonEmptyList. I haven't mentioned the Applicative, so, If you see that we need to detail a little bit about it or If I've made some mistake in the definition, please let me know :) (PS: I think that the Wikipedia quote should fit well here and also the reference to the Semigroup docs).
I definitely don't know how to implement the example of .toValidated/.toValidatedNel in a fail-slow manner. I've tweaked a bit that section (renamed to Going back and forth) adding the .toEither combinator. If you have a suggestion for the fail-slow example, please let me know.
If not, I can delete that part.

LukaJCB · 2017-09-12T10:38:23Z

About your 3rd point, why don't you just convert some of your Either example using toValidatedNel, e.g. validateUserName. That should work, right? :)

LukaJCB · 2017-09-12T10:41:02Z

docs/src/main/tut/datatypes/validated.md

+
+Typically, you'll see that `Validated` will be accompanied by a `NonEmptyList` when it comes to accumulation. The thing here is that you can define your own accumulative data structure and you're not limited to the aforementioned construction.
+
+For doing this, you have to provide a `Semigroup` instance. `NonEmptyList`, by definition has its own `Semigroup`. For those who don't know what a `Semigroup` is, let's see a simple example.


This is really great, thank you! However, you didn't really include an example?

It was more about how ap works for Invalid, but I can also provide a simple example of Semigroup. What do you think about giving the example first and then point to the actual behavior, as it is written now?

LukaJCB · 2017-09-12T10:42:20Z

docs/src/main/tut/datatypes/validated.md

+
+#### Accumulative Structures
+
+According to [Wikipedia](https://en.wikipedia.org/wiki/Semigroup):


Instead of linking to wikipedia, I think the link to cats documentation should be enough :)
Maybe we could just say " For those who don't know what a Semigroup is, you can find out more here."

What do you think? :)

Sounds good! I'll fix it.

AlejandroME · 2017-09-12T11:51:34Z

@LukaJCB Yes! Sorry, I didn't figure it out! I'll work on this :)
Thanks for the suggestion!

peterneyens

Thanks already @AlejandroME !

I left a few comments.

peterneyens · 2017-09-12T21:58:18Z

docs/src/main/tut/datatypes/validated.md

+
+sealed trait FormValidator{
+  private def validateUserName(userName: String): Either[DomainValidation, String] =
+    if (userName.matches("^[a-zA-Z0-9]+$")) Right(userName) else Left(UsernameHasSpecialCharacters)


You could use Either.cond instead of if (...) Right(...) else Left(...).

This is new for me! I'll change it in all the conditions :)

peterneyens · 2017-09-12T22:02:58Z

docs/src/main/tut/datatypes/validated.md

+
+def validateUserName(userName: String): Validated[DomainValidation, String] = {
+  if (userName.matches("^[a-zA-Z0-9]+$")) Valid(userName) else Invalid(UsernameHasSpecialCharacters)
+}


Maybe reuse the Either methods here and call toValidated ?

Good suggestion. I'll take a look at it!

peterneyens · 2017-09-12T22:04:14Z

docs/src/main/tut/datatypes/validated.md

+```tut:silent
+import cats.data._
+import cats.data.Validated._
+import cats.implicits._


You don't need to import these again.

peterneyens · 2017-09-12T22:06:04Z

docs/src/main/tut/datatypes/validated.md

+
+sealed trait FormValidatorNel {
+
+  type ValidationResult[A] = Validated[NonEmptyList[DomainValidation], A]


Since you are using validNel and invalidNel below, I think you can use ValidatedNel here as well.

peterneyens · 2017-09-12T22:23:46Z

docs/src/main/tut/datatypes/validated.md

+}
+```
+
+We've omitted the complete implementation because our focus here is the case in where you need to append (that's the function of this method) two failures. Note the `implicit EE: Semigroup[EE]` parameter and the usage of its `.combine` operation. In the case of `NonEmptyList`, we're talking about a `List`, with certain properties that allow us to _combine_ (append) more than one element to it. That's because, apart from the fact that it is a `List`, it also has an instance of `Semigroup`, telling it how to operate with the accumulation.


I find this section a bit confusing.

Maybe something like :

We've omitted the complete implementation and only show the part where two failures are combined using the combine method of the Semigroup instance of EE.

I don't think we need the part about NonEmptyList. Maybe you can add an example of NonEmptyList's Semigroup to the Accumulative Structures section ?

Something like :

NonEmptyList.one("error 1") |+| NonEmptyList("error 2", "error 3") "error 1".invalidNel[Int] |+| "error 2".invalidNel ("error 1".invalidNel[Int], "error 2".invalidNel[Int]).mapN(_ + _)

peterneyens · 2017-09-12T22:27:25Z

docs/src/main/tut/datatypes/validated.md

+### Going back and forth
+
+cats offer you a nice set of combinators to transform your `Validated` based approach to an `Either` one and vice-versa.
+Please note that, if you're using an `Either`-based approach as seen in our first example and you choose to convert it to a `Validated` one, you're constrained to the fail-fast nature of `Either`, but you're gaining a broader set of features with `Validated`.


What do you mean with

you're gaining a broader set of features with Validated.

This is the section in where I have doubts about it. I was trying to express that, coming from Either to Validated, the person will gain more combinators (Features), even if you transfer the Either behavior (fail-fast) to Validated.

With your previous comments, I have an idea for simplifying this section (or even delete it). Let me work on it and I'll reach you out again :)

peterneyens · 2017-09-12T22:30:20Z

docs/src/main/tut/datatypes/validated.md

+Our data will be represented this way:
+
+```tut:silent
+case class RegistrationData(username: String, password: String, firstName: String, lastName: String, age: Int)


Maybe add final here. Probably good to show best practices in the documentation?

- Added `final` modifier to `RegistrationData` case class. - Changed if-else in `Either` example to `Either.cond`. - Reused `Either` example validations in `Validated` (non-compiling) approach with `.toValidated`. - Deleted redundant imports for tut. - Changed `Validated` of a `NonEmptyList` for `ValidatedNel` type alias. - Deleted 'disclaimer' about converting from `Validated` to `Either`, losing the fail-slow functionality. - Added `Semigroup` example based on the Peter's one but in the context of the example provided here. - Fixed some typos and grammar errors. - Pointed out to the cats `Semigroup` documentation instead of Wikipedia.

AlejandroME · 2017-09-17T19:06:31Z

Hello, @LukaJCB and @peterneyens!

I've worked on your suggestions. Some notes:

I've deleted the ambiguous part about the conversion between Validated and Either, "losing the fail-slow feature but gaining some other combinators". With the suggestions of @peterneyens, since the beginning of the docs I've used .toValidated for reusing the validation methods and then at the end I've used .toEither on the ValidatedNel example.
I've followed the advice of Peter about the Semigroup example and I just used the example that he provided about NonEmptyList, but with the validations developed through this docs.
I've fixed some typos and fixed some other things.

I'll stay tuned for your comments about this.

Thanks!

kailuowang · 2017-09-26T11:55:08Z

Looks like all comments are addressed. LGTM thanks so much @AlejandroME !!

AlejandroME · 2017-09-26T11:59:08Z

Thank you so much for all the feedback received! I'm very happy to contribute to the project! 👍

alejandromarin added 2 commits August 27, 2017 16:45

[WIP] Beginner-Friendly Validated docs

ef08641

This commit is the first approach to address typelevel#1678, developing the form-validation example.

Beginner-friendly example of validated.

650c537

This commit contains a simple form-validation example and a couple of approaches making use of `Validated`. It aims to solve typelevel#1678

kailuowang added the documentation label Sep 9, 2017

LukaJCB reviewed Sep 10, 2017

View reviewed changes

LukaJCB reviewed Sep 11, 2017

View reviewed changes

LukaJCB reviewed Sep 12, 2017

View reviewed changes

peterneyens reviewed Sep 12, 2017

View reviewed changes

AlejandroME mentioned this pull request Sep 25, 2017

Documentation megathread #1801

Open

70 tasks

LukaJCB approved these changes Sep 26, 2017

View reviewed changes

kailuowang merged commit dc8637d into typelevel:master Sep 26, 2017

AlejandroME deleted the validated-beginners-doc branch September 26, 2017 11:59

kailuowang added this to the 1.0.0-RC1 milestone Oct 13, 2017


		### Meeting applicative

		We have to look into another direction: a for-comprehension plays well in a fail-fast scenario, but the structure in our previous example was designed to catch one error at a time, so, our next step is to tweak the implementation a bit.


		Typically, you'll see that `Validated` will be accompanied by a `NonEmptyList` when it comes to accumulation. The thing here is that you can define your own accumulative data structure and you're not limited to the aforementioned construction.

		For doing this, you have to provide a `Semigroup` instance. `NonEmptyList`, by definition has its own `Semigroup`. For those who don't know what a `Semigroup` is, let's see a simple example.


		#### Accumulative Structures

		According to [Wikipedia](https://en.wikipedia.org/wiki/Semigroup):


		sealed trait FormValidatorNel {

		type ValidationResult[A] = Validated[NonEmptyList[DomainValidation], A]

Validated beginners doc #1903

Validated beginners doc #1903

Conversation

AlejandroME commented Sep 9, 2017

codecov-io commented Sep 9, 2017 • edited Loading

Codecov Report

LukaJCB commented Sep 10, 2017

AlejandroME commented Sep 10, 2017

AlejandroME commented Sep 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlejandroME commented Sep 10, 2017

LukaJCB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlejandroME Sep 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlejandroME Sep 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlejandroME Sep 11, 2017 • edited Loading

Choose a reason for hiding this comment

AlejandroME commented Sep 12, 2017

LukaJCB commented Sep 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlejandroME commented Sep 12, 2017

peterneyens left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlejandroME commented Sep 17, 2017 • edited Loading

kailuowang commented Sep 26, 2017

AlejandroME commented Sep 26, 2017

codecov-io commented Sep 9, 2017 •

edited

Loading

AlejandroME commented Sep 10, 2017 •

edited

Loading

AlejandroME Sep 11, 2017 •

edited

Loading

AlejandroME Sep 11, 2017 •

edited

Loading

AlejandroME Sep 11, 2017 •

edited

Loading

AlejandroME commented Sep 17, 2017 •

edited

Loading