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

Updated WeePickle readme.md around Defaults, Options, @dropDefault, and Null Handling #79

Open
wants to merge 3 commits into
base: v1
Choose a base branch
from

Conversation

Outwise
Copy link

@Outwise Outwise commented Jun 17, 2021

  • added IDE "bsp" dir to .gitignore; IDE specific files shouldn't be in repos
  • Overhauled Defaults, Options, @dropDefault, and Null Handling. Condensed sections, then expanded examples to show typical use cases.
  • some consistency tweaks, minor corrections

TODO

  • could use help confirming changes are comprehensive,
  • no missed copy/paste errors,
  • accurate assumptions on expected results

…in repos

+ Overhauled Defaults, Options, @dropDefault, and Null Handling. Condensed sections, then expanded examples to show typical use cases.
+ some consistency tweaks, minor corrections
```

If attempting to deserialize JSON to a field that has no defaults, and the JSON value is invalid (e.g. missing, or `null`), the operation will throw an Abort Exception.
Copy link
Contributor

Choose a reason for hiding this comment

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

Mostly correct. null handling is up to the To. Most will throw, but Option, Array, and others may override visitNull. The case class macro itself doesn't do any special null-handling.

case class Maybe1(i: Option[Int])
object Maybe1 {
implicit val rw: FromTo[Maybe1] = macroFromTo
case class DfltOpt(i: Option[Int]) // equivalent to 'i: Option[Int] = None` for weepickling
Copy link
Contributor

Choose a reason for hiding this comment

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

nice clarification.

So putting `@dropDefault` at the class level will apply to all `Option` types in the class, whether a default is provided explicitly or not.
### Explicit Defaults

An `Option` will have an [implicit default of `None`](#Nones-&-Nulls), unless you provide an explicit default ([Defaults](#Defaults)):
Copy link
Contributor

Choose a reason for hiding this comment

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

Broken links here.


### Option Defaults

`Option` works the same way, except there is always an assumed implicit `None` default, if an explicit default is not provided. See [Option Defaults & Nulls](#Option-Defaults-&-Nulls).
Copy link
Contributor

Choose a reason for hiding this comment

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

Broken link here.

@@ -115,81 +115,184 @@ Case classes are serialized using the `apply` and `unapply` methods on their com
Anything else is not supported by default, but you can add support using Custom Picklers.

## Defaults
If a field is missing upon deserialization, weePickle uses the default value if one exists.
If a field is missing upon **deserialization**, weePickle uses the default value if one exists:
Copy link
Contributor

Choose a reason for hiding this comment

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

I know deserialization and serialization were already here, but they're a bit misleading. All of these behaviors have to do with case classes rather than serialization formats. For example, you can do FromJson("{}").transform(ToMsgPack.string), which is both deserialization and serialization, but none of this will apply.

More README surgery would be welcome if interested. Also, totally fine to leave for a separate PR.

.gitignore Outdated Show resolved Hide resolved
Copy link
Contributor

@htmldoug htmldoug left a comment

Choose a reason for hiding this comment

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

Thanks so much for sprucing up the docs!

John Gosling and others added 2 commits June 17, 2021 11:57
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

Successfully merging this pull request may close these issues.

4 participants