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

Add option to generate a function rather than global variable #88

Open
invidian opened this issue Sep 10, 2020 · 4 comments
Open

Add option to generate a function rather than global variable #88

invidian opened this issue Sep 10, 2020 · 4 comments

Comments

@invidian
Copy link

It would be great to have another field in Options struct called FunctionName, which would generate a function returning http.FileSystem, rather than a function assigned to global variable, as globals are magic and should not be used whenever possible (though a function in global is not that bad 😄).

@dmitshur
Copy link
Member

Can you please expand a bit on the motivation? What would you do differently if this were implemented?

Is this a feature request to be able to defer the cost of initialization of the virtual filesystem to happen on first call, rather than at program start up time?

@invidian
Copy link
Author

Hmm, to be honest, my main motivation is only following Go best practices, which recommends avoid global variables. Also, I would like to understand the reasoning behind chosing:

var vfsgenAssets = func() http.FileSystem {

over

func vfsgenAssets() http.FileSystem {

Are they exactly the same?

@dmitshur
Copy link
Member

The best practice recommends avoiding global variables when it is possible and when it makes sense. It doesn't seem very applicable in this situation.

To answer your question, let me be more precise. If you're asking if there's a difference between:

var vfsgenAssets = func() http.FileSystem {
    ...
}()

Which is the code vfsgen generates today, and:

func vfsgenAssets() http.FileSystem {
    // ...
}

Then they are not the same. The () at the end of the first block makes a big difference. In it, a variable with type http.FileSystem is initialized and assigned to the vfsgenAssets variable exactly once.

In the second block, vfsgenAssets is a function that returns a variable with type http.FileSystem. Each time it's called, it does the work of initializing a new copy of that variable.

(If () were omitted, they would be closer to the same, with the difference that a function is immutable, whereas a variable can be modified.)

@invidian
Copy link
Author

Then they are not the same. The () at the end of the first block makes a big difference. In it, a variable with type http.FileSystem is initialized and assigned to the vfsgenAssets variable exactly once.

Oh, I didn't notice that this anonymous function is actually called at the end! In this case, it definitely make sense to make it a function IMO, as currently vfsgenAssets, if exported, is a global variable, so one can just remove all values from it. I think from it's nature generated assets should be immutable.

(If () were omitted, they would be closer to the same, with the difference that a function is immutable, whereas a variable can be modified.)

Good point, TIL.

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

2 participants