There's so much written about style guidelines and the official documentation, and the general consensus is common sense, really.
But sometimes things like this come up: CompareBase64HashAndPassword
contains all of the information you need to know what the method does, but it, at least to me, is a very long name. We could shorten it to CmpB64HashPass
but CmpB64HashPass
could mean the same thing to one person, or it could mean "compute the base64 hash from this password", until they see that the argument takes two strings. But then they could think "what if that's the password and salt?"
Then they have to go read the documentation or the actual function to know that it's decoding the base64URL-encoded hash, and then comparing that to that to a recomputed hash from the given password.
But the first method name just seems too long.
评论:
relvae:
glacials:Keep it simple, first option is easily more preferable imo.
Another consideration is the context for which the function resides. For example if the package was called
password
then you could shorten the name to simplypassword.CompareBase64Hash()
. Another case could be where you use the function parameters to provide additional context so thatCompareBase64(hash, plaintext string)
.Go with whatever feels most intuitive
epiris:A function is more than a name, it's the whole signature.
type hash string type password string func (h hash) Matches(p password) bool { ... }
This is an extreme example, but don't feel afraid to lean on your arguments for descriptive power.
By the way, I hope your passwords are not just converted into base 64. That's not a hash, or even encryption, just another way of storing plain text.
nyxxxie:In most Go code I see the context describes the object as well, such as the package name and type. Such a name would be redundant in a well formed and designed package. This is why package names like “utils” are discouraged and using the Type system properly is important to prevent programmer error.
Here for example you are having to describe input invariants by naming due to ambiguity between string and a base64 encoded hash string and a Password string. So a type safe interface may instead look like:
package authorize // enforce type safety type Equaler interface { Equal(Equaler) bool } type Password string // impl Equaler type Hash string; // impl Equaler
Then anything that returned a Base64Hash would return a Hash, I.e. maybe you would have a Hash() function on your Password type to derive a Hash type. Then comparing looks like Hash(hashFromDB).Equal(Password(requestPassword).Hash())
Obviously this abstraction is going wayyyy to far. Strings are comparable so you should just have a Hash type and compare them. But point was to illustrate how naming, using the type system and sharing interfaces qualifies objects to prevent obnoxious naming.
xiegeo:Out of curiosity, what is this method being used for?
NatoBoram:
Verify(hash, password string) bool
That's what I would prefer.
You should also add salt and algorithm arguments too.
evmar:Basically, you want
password_verify
.I wish this and
password_hash
were implemented by default in every single languages. :/
https://github.com/golang/go/wiki/CodeReviewComments#variable-names
discusses this a bit:
The basic rule: the further from its declaration that a name is used, the more descriptive the name must be. For a method receiver, one or two letters is sufficient. Common variables such as loop indices and readers can be a single letter (i, r). More unusual things and global variables need more descriptive names.
