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

Focus operator naming #26

Closed
kenbot opened this issue Jan 17, 2021 · 3 comments
Closed

Focus operator naming #26

kenbot opened this issue Jan 17, 2021 · 3 comments
Labels
question Further information is requested

Comments

@kenbot
Copy link
Collaborator

kenbot commented Jan 17, 2021

Given that we already have (or plan to have) named methods like some and each that compose those respective optics onto an optic, shouldn't we keep that naming inside the Focus macro rather than having novel synonyms like ? or *?

Point:

Consistency is important. The same concept should not have any more than one name.

Counterpoint:

It will be hard to distinguish operators from regular fields, looking at the expression, leading to confusion in users. We are effectively asking them to memorise all the operators in order to use or understand Focus.

Counter-counterpoint:

"at" and "index" are already common words, and the same as our methods on the optics classes. Why have some the same and not others? We can make the Focus API discoverable by liberally printing usage instructions in compile error messages.

Counter-counter-counterpoint:

What do we do with naming collisions? At least special operators are less likely to collide with case class field names.

Counter-counter-counter-counterpoint:

Yes, but it's not impossible. We need a name collision policy anyway. Would compiler warnings be too annoying? Perhaps we could offer alternative names like __some__ or __index__ so that users don't get stuck. Or perhaps we could offer another operator field(_.some) to make a field access irrefutable.

@kenbot kenbot added the question Further information is requested label Jan 18, 2021
@kenbot
Copy link
Collaborator Author

kenbot commented Jan 20, 2021

Based on a suggestion from @julien-truffaut, I suggest the following semantics in Focus:

  • Keywords with exactly the same names as their Monocle cognates, if possible. No special symbols, no aliases. I think we're up to: some, each, index, at, as, field.
  • If a name is a case class field, but is not a keyword, Focus will construct a Lens for it in the usual way.
  • If a name is not a case class field, and is not a keyword, we expect that the Scala typechecker will kill it before Focus gets to run.
  • If a name is not a case class field, but is a correctly used keyword, Focus should apply the keyword as intended.
  • If a name is not a case class field, but is an incorrectly used keyword, Focus should throw a compiler error that explains the correct usage of the keyword.
  • If a name is a case class field, and is also a keyword, then Focus should throw a compiler error explaining that fields with the same name as keywords cannot be directly used, and directing them to wrap it the field keyword, with an example usage. (eg Focus[User](_.address.field(_.some)))
  • If a name is wrapped in the field keyword, Focus will construct a Lens for it in the usual way.

To do this, it would be useful for the DSL keywords:

  • To be universal extensions, so that Focus can decide to handle them, rather than the Scala typechecker
  • Not to require a separate import. Once someone is using Focus, they are using Focus.
  • Not to be visible outside the scope of the macro.
    We might need to experiment a bit to find out what is possible there.

@kenbot
Copy link
Collaborator Author

kenbot commented Jan 21, 2021

If we really like * and ?, we could always make them the methods on the Each and Some typeclasses, to be consistent...

@julien-truffaut
Copy link
Member

julien-truffaut commented Jan 21, 2021

IMO, we should aim for one name so that users don't have to argue, shall we use some or ? in pull requests. It will also simplify the documentation and make examples more consistent.

But we definitely need to consider name collision. Maybe we should define test cases, e.g.

case class Foo(bar: String, index: String, elems: List[String]) {
  def index(key: Int): String = elems(key)
}

object Foo {
  implicit val indexEv: Index[Foo, Int, String] = ...
}

 // Generate a lens to the field bar
Focus[Foo](_.bar)   

// generate a lens to the field index OR 
// fail because there is ambiguity? e.g. use Focus[Foo](_.field(_.index)) 
Focus[Foo](_.index) 

// Fail no field fizz in Foo
Focus[Foo](_.fizz)         

// generate an Optional using Foo.indexEv  OR
// Fail index is a method of Foo, if it fails how do we force the use of Index instance? __index__(3)?
Focus[Foo](_.index(3))

@kenbot kenbot closed this as completed Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants