Unexport default package global variables and Blocklist function

[peterhellberg]

I took a quick look at this library and was surprised to see exported package level variables used for default values, this is just a quick PR to remove that surface area from the library.

If this was intentional for some reason then feel free to close this PR 😃

[!NOTE] If you agree, then I'd like to (in another PR) reconsider the use of pointers for the options fields, as they somewhat makes them inconvenient to use.

[4kimov]

Hi @peterhellberg, thanks for the PR. They were exported for a reason: For example, if there's a word that they would like to add to the blocklist, they could append their custom word to the default blocklist, as opposed to having to pass the whole thing manually. The URL-safe alphabet is exported in case the user would like to programmatically shuffle it. Exporting min length is mainly just so it's inline with the others.

Maybe I'm not seeing something, is there a better way to allow that?

[peterhellberg]

Yes, as currently implemented, they would not be able to change what is returned from the package level function Blocklist (since it is an inline slice being returned), but as you mentioned a user could call that exported function and then append to what is returned.

I would probably instead expose a constructor function for a blocklist based on the default slice + what is passed in by the end user. (If this is a common enough use case that is)

I have some other concerns as well, maybe it is better to open issues rather than PRs in order to clarify some things? (for example why a []uint64 is looped over to check if each uint64 is in fact a uint64 value between (and including) 0 and math.MaxUint64, something that will always be the case for a slice of 1 or more uint64 values)

[4kimov]

Re: looped over, yes sounds like an oversight that should be removed.

Re: constructor/blocklist. Basically the spec says 3 scenarios should be supported:

1. By default, the default blocklist should be used
2. A user should be able to provide an empty blocklist to not use the blocklist feature at all
3. Finally, a user should be able to provide a custom blocklist (this is where it might be helpful to expose the default blocklist in case they'd like to improve it).

If I understood you correctly, you're suggesting creating functions that allow all of the above user-cases, while keeping the default blocklist private? If so, I can see the benefit if that's the idiomatic Golang way, but I do wonder if we'd be unnecessarily restricting end-user's access to the default vars (for whatever reasons they might have; eg: remove only leetspeak words and keep the rest)?

[peterhellberg]

One primary concern I have is that it is (fairly) commonly agreed that exported package level variables should be avoided in Go, if at all possible (since they pollute the API and restrict you from making some changes in the future.)

I'll do some minor updates to this PR to illustrate a bit what I'm suggesting in regards to a constructor function for Blocklist