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

Optional non-empty fields in patched class overwritten by empty optional fields of patcher #69

Closed
AlifCor opened this issue Jul 11, 2018 · 7 comments

Comments

@AlifCor
Copy link

AlifCor commented Jul 11, 2018

Consider the following:

case class User(name: Option[String], age: Option[Int])

case class UserPatch(name: Option[String], age: Option[Int])

val user = User(Some("John"), Some(30))
val userPatch = UserPatch(None, None)

user.patchWith(userPatch) == user //expected behaviour, but *false* 
user.patchWith(userPatch) == User(None, None) //true

I would expect that, if the optional fields of the patchers are empty and those of the patched class are not, the latter ones should remain. Is the current behaviour the expected one ?

@krzemin
Copy link
Member

krzemin commented Sep 8, 2018

Considering current implementation, this is expected behavior. If the types of corresponding fields are the same, value from the patch is taken.

However there is some support for the optional fields in the patch. Considering you have a field of type T in the object and Option[T] in the patch, the value from the patch is taken only, when it's defined.

It can be demonstrated with slight modification of your example:

case class User2(name: String, age: Int)

> val user2 = User2("John", 30)
user2: User2 = User2("John", 30)

> user2.patchWith(UserPatch(None, None)) == user2
res: Boolean = true

Or even other way around, wrapping patch fields in Option:

case class UserPatch2(name: Option[Option[String]], age: Option[Option[Int]])

> user.patchWith(UserPatch2(None, None)) == user
res: Boolean = true

@AlifCor
Copy link
Author

AlifCor commented Oct 6, 2018

However there is some support for the optional fields in the patch. Considering you have a field of type T in the object and Option[T] in the patch, the value from the patch is taken only, when it's defined.

Do you think it would make sense to extend that support to the case I demonstrated ? Namely, you have a field of type Option[T] and Option[T] in the patch, and have the value from the patch taken only if it's defined.

@krzemin
Copy link
Member

krzemin commented Oct 6, 2018

Well, both semantics may have perfect sense in different contexts. I think it makes sense to support also your use case under a flag. Would you like to craft a PR?

@AlifCor
Copy link
Author

AlifCor commented Dec 9, 2018

Thanks, I'll take a look once I have some time.

@lloydmeta
Copy link

I just ran into this one, and thought I could get away with defining a local Patcher[Option[A], Option[A]] that incorporates fallback-like semantics, but no go. I guess the macro for generating Patchers doesn't search for implicits?

@ import $ivy.{`io.scalaland::chimney:0.3.0`}
import $ivy.$

@ import io.scalaland.chimney.dsl._
import io.scalaland.chimney.dsl._

@ import io.scalaland.chimney._
import io.scalaland.chimney._

@ implicit def optionFallbackPatcher[A] = new Patcher[Option[A], Option[A]] {
    def patch(o: Option[A], patch: Option[A]): Option[A] = o.orElse(patch)
  }
defined function optionFallbackPatcher

@ final case class Data(o: Option[Int])
defined class Data

@ Data(Some(1)).patchWith(Data(None)) // would have expected res5 = Data(Some(1))
res5: Data = Data(None)

@krzemin
Copy link
Member

krzemin commented Jan 18, 2020

Discussion on the same topic continued in #123.

@krzemin
Copy link
Member

krzemin commented Jan 21, 2020

Resolved in #131.
@lloydmeta you're right that current patchers macro doesn't look for implicits. I created new issue (#133) which aims to resolve it.

@krzemin krzemin closed this as completed Jan 21, 2020
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

3 participants