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

wire: do not use global variables for all wire.Value expressions #185

Open
rogpeppe opened this issue May 25, 2019 · 4 comments
Open

wire: do not use global variables for all wire.Value expressions #185

rogpeppe opened this issue May 25, 2019 · 4 comments
Labels
enhancement good first issue
Milestone

Comments

@rogpeppe
Copy link

@rogpeppe rogpeppe commented May 25, 2019

One of the specific advantages of the wire approach is that it generates code that's relatively readable compared to reflect-based equivalents. When wire.Value is used on a small by-value type, there's no need for the value to live in a global variable - the expression could instead be used literally inside the generated code, which would make the code easier to follow, and more similar to the code that one might write by hand.

@zombiezen zombiezen added this to the Unplanned milestone Jun 7, 2019
@zombiezen zombiezen added the enhancement label Jun 7, 2019
@zombiezen
Copy link
Collaborator

@zombiezen zombiezen commented Jun 7, 2019

The reason we have the semantics we have now for wire.Value is that we want to be confident that the expression will evaluate to the same value that was expressed at the wire.NewSet's evaluation. This keeps the behavior of wire.Value largely unsurprising, and amenable to usage in a runtime DI version if needed at some point.

It sounds like the request here is that the generated code in some circumstances (like when using a constant expression, for example) could be more efficient, since the time that the expression is evaluated wouldn't matter. That seems reasonable to me, but I think we can add this feature in a backward-compatible way later.

@rogpeppe
Copy link
Author

@rogpeppe rogpeppe commented Jun 8, 2019

Yes, but it's not just about efficiency. One of the selling points of wire is that the generated is a readable representation of the dependency setup. This is one thing that makes the code less idiomatic/readable

@zombiezen
Copy link
Collaborator

@zombiezen zombiezen commented Jun 11, 2019

Agreed. To be clear: I think this is a good idea, but we don't have bandwidth to fix it right now. Happy to review PRs to fix.

@zombiezen zombiezen added the good first issue label Jun 11, 2019
@j4ng5y
Copy link

@j4ng5y j4ng5y commented Jun 25, 2019

I'll take a stab at this.

@zombiezen zombiezen removed their assignment Jul 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement good first issue
Projects
None yet
Development

No branches or pull requests

3 participants