8000 Rename ZLayer to ZServiceBuilder by adamgfraser · Pull Request #5815 · zio/zio · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Rename ZLayer to ZServiceBuilder #5815

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 16 commits into from
Nov 13, 2021
Merged

Rename ZLayer to ZServiceBuilder #5815

merged 16 commits into from
Nov 13, 2021

Conversation

adamgfraser
Copy link
Contributor
@adamgfraser adamgfraser commented Oct 22, 2021 •
edited
Loading

Resolves #5805.

The ZLayer name really only makes sense in terms of the "onion architecture" and representing different "layers" of the onion (e.g. the Github service might translate from the lower level domain of HTTP to the higher level domain of the Github API). While this is a very insightful way to view program architecture, it requires you to know this particular concept which is an obstacle to usability and teachability because many people may not.

This PR explores instead naming it ZDeps for dependencies, a bundle of dependencies that you provide to your application.

@adamgfraser adamgfraser requested a review from iravid as a code owner October 22, 2021 03:45
@adamgfraser adamgfraser requested a review from jdegoes October 22, 2021 20:58
@guersam
Copy link
Contributor
guersam commented Oct 29, 2021

IMHO, 'dependency' seems to capture only half (input) side of ZLayer, because ZLayer also has the output side that enables building and launching itself directly. ZDeps#build and ZDeps#launch sound a bit unnatural to me in that sense.

If I must choose one alternative name to ZLayer I'd suggest ZModule, because we're already introducing ZLayer with the 'module pattern'. A downside is that we probably have to peek another name than ZIO#provideModule.

@alphaho
Copy link
Contributor
alphaho commented Nov 12, 2021 •
edited
Loading

@guersam , how about a name like ZBuilder (or even ZDepsBuilder)? A builder requires some setup/configuration, and can build an object out of it. That may capture the both input and output side of ZLayer.

@regiskuckaertz
Copy link
Member

Throwing my hat in the game with ZRequire for the duality with provide 😁

@ghostdogpr
Copy link
Member

Throwing my hat in the game with ZRequire for the duality with provide 😁

But provide doesn't actually require a ZLayer, provideLayer does 🤪

@regiskuckaertz
Copy link
Member

True 😂 I think the profunctor property is not exploited anymore, iirc @adamgfraser told me this wasn't a pattern that was encouraged so much anymore, and I find it has been a challenge to teach layers and the R parameter as two separate things. So perhaps an opportunity to make layers or whatever their name more natural to express.

@guersam
Copy link
Contributor
guersam commented Nov 12, 2021 •
edited
Loading

Frankly, I often get confused of when to encode the dependency in R type or to use constructor injection with ZLayer, even though I'm familiar with the onion architecture concept.

I think we need to gather the best practices for this first, then upon a deeper understanding we may find better name for ZLayer or reach a consensus to keep it as it is.

@jdegoes
Copy link
Member
jdegoes commented Nov 12, 2021

After a lot of thought, the I've grown to like ZServiceBuilder.

Why? Because the terminology seems to be consistent:

  • You access services from the environment with ZIO.service
  • You create service definitions with interfaces / traits
  • You implement services with classes
  • You build services from their dependencies with service builders
  • Service builders describe how to build services from their dependencies

As part of this, we would change "Module Pattern" to "Service Pattern", thus consolidating a lot of different terms under the "service" umbrella. /cc @khajavi

@jdegoes
Copy link
Member
jdegoes commented Nov 12, 2021

@guersam I agree there is confusion here. @khajavi Let's meet to discuss best practices around R versus Layer so we can thoroughly document it on zio.dev.

@khajavi
Copy link
Member
khajavi commented Nov 12, 2021

@jdegoes
I definitely agree with ZServiceBuilder and Service Pattern as we have discussed before.

In addition, regarding this term consolidation, I propose renaming the Has data type to the Service or Srv. Therefore, the ZServiceBuilder[Srv[A] with Srv[B], Throwable, Srv[C]] makes more sense than the has terminology for newcomers.

This would be consistent with the provideService API which indicates that we should provide it an Srv[A] instead of raw A data type.

@adamgfraser
Copy link
Contributor Author

I like it too. Will update accordingly.

@adamgfraser adamgfraser changed the title Rename ZLayer to ZDeps Rename ZLayer to ZServiceBuilder Nov 12, 2021
@adamgfraser
Copy link
Contributor Author

@khajavi I like the direction you are going in renaming Has. How would you propose describing the type inside the Has. For example if we have Has[Console] I would normally describe Console as a service and Has as a tool that we use to combine services. If we use Service[Console] that seems a bit more unclear. Potentially we could say that Service denotes that Console is a service?

@adamgfraser
Copy link
Contributor Author

@jdegoes This is ready for review.

jdegoes
jdegoes previously approved these changes Nov 13, 2021
View reviewed changes
@jdegoes
Copy link
Member
jdegoes commented Nov 13, 2021

@adamgfraser One minor question:

Should it be provideService (singular), or provideServices (plural)? I would have slightly leaned toward the latter, but I saw you went with the former. What are your thoughts?

@jdegoes
Copy link
Member
jdegoes commented Nov 13, 2021

The Has has existing usage inside scala code bases, e.g. HasFoo is usually a trait that exposes a foo, for purposes of withing all the containers together. I agree with @adamgfraser that Service[Foo] would be a bit confusing because it equivocates on what is the service.

@adamgfraser
Copy link
Contributor Author

@jdegoes I wondered about that too. I went with provideService because you can potentially provide one or more services, ZServiceBuilder is itself singular, and it provides a consistent namespace for everything "service related".

However, thinking about it more I like making it plural because it creates a distinction between operators like ZIO#updateService (update a single service) and ZIO#provideServices (provide zero or more services if you count the fact that a ServiceBuilder can be built purely for its effects).

Will change.

@adamgfraser adamgfraser requested a review from jdegoes November 13, 2021 13:08
@jdegoes jdegoes merged commit 4b44cc4 into zio:series/2.x Nov 13, 2021
@adamgfraser adamgfraser deleted the zdeps branch November 13, 2021 14:10
@octavz
Copy link
Contributor
octavz commented Nov 14, 2021

Late to the party, I didn't know why I hated so much reading about this change. Thinking about it I figured out that apart from sounding like a a "Java something" that I ran from and came to ZIO, it is very long. Given the invariance of Has, ZLayer is the most typed thing when using ZIO, apart from ZIO.

You took something not great to type and made it worse, ZServiceBuilder[Any, Throwable, Service] , that's a mouthful, also is nothing like a builder so this will not help anyone coming from Java expecting a Builder to be a builder.

I don't know if is better for new people coming to ZIO, I helped many to adopt ZIO and the name Layer was the least of their problems, but for me and my codebase will be a hassle to use the new name and adopt a new vocabulary.

@adamgfraser
Copy link
Contributor Author

@octavz Sorry to hear that!

We're always trying to strike the right balance for all users. There are definitely some people who found the concept of layers confusing and it really did only have a meaning within the concept of an "onion pattern" architecture which is great but not everyone knows.

As is often the case with naming I don't think there was a perfect solution here. As you can see we went through a variety of alternatives that people proposed and on balance this seemed to be the best alternative. It does exactly what it says, which is build the services your application needs. You are right it is longer but we think conceptual clarity wins out over terseness, especially with features like autocomplete in modern IDEs.

One thing that did jump out at me is your statement that this is the second most frequent data type you type. That is very surprising to me. Use cases are definitely going to vary but I find it is typically one of the ones I write least frequently.

Anyway, definitely appreciate your perspective and hope that over time with other features in ZIO 2.0 like automatic construction of the dependency graph and a more structured way of defining services we can win you over and make building your application's dependencies as easy as possible.

@octavz
Copy link
Contributor
octavz commented Nov 15, 2021

@adamgfraser

I am not complaining I will just adapt, but I am also surprised by your "surprise" :),

Unless we are using this completely wrong:

You have to type ZLayer every time you make a layer (twice every time: once for the type since it will always infer with the sub type LiveXXX and once for ZLayer.succeed or fromfunction or friends).

At first sight it might not be a lot but think about the tests, every test will stub and every stub has to become a layer, every test has at least one stub which makes "ZLayer" or variants basically the most used thing in your code.

Also if we follow conventions any vals that now contain layer would have to contain serviceBuilder.

For a small module:

>  ag --stats --scala 'ULayer|ZLayer|URLayer'

550 matches
44 files contained matches

compare to ZIO usages

ag --stats --scala -c 'RIO|Task|ZIO'
677 matches
50 files contained matches

Also IDEs for Scala are slower than typing most of the time so they are not big help there, IntelliJ hardly works with ZIO, Metals "almost" works but more times it doesn't than it does.

@joroKr21
Copy link
Contributor

Happy this got reverted but for posterity I wanted to share another opinion. ZLayer is not a simple builder to me because it caches services with the same type. Maybe ZDependencyGraph would make more sense because it really represents a graph of your dependencies - every service type is its own node with edges between services representing a dependency. But still I like ZLayer more anyway - two words are too much for such a core concept.

@kluen kluen mentioned this pull request Apr 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdegoes jdegoes jdegoes approved these changes

@iravid iravid Awaiting requested review from iravid

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@adamgfraser @guersam @alphaho @regiskuckaertz @ghostdogpr @jdegoes @khajavi @octavz @joroKr21 @atooni
0