Crystal: Crypto::Bcrypt::Password#== is not commutative

[asterite]

As discussed in the forums, it's a bit unintuitive that given password : Crypto::Bcrypt::Password and string : String, password == string will work but string == password won't.

Two solutions:

1. Add an overload String==#(other : Crypto::Bcrypt::Password) so it's commutative
2. Use a different method name. Ruby's BCrypt uses is_password? as an alias, but verify, check or matches? might be equally good.

I'm not sure which approach is better. I think the second one is better because it's also more type-safe. You can do password == 1.23 and it will still compile, but it's obviously incorrect. You can even do password == password and it will return true, but it makes little sense.

Thoughts?

[Sija]

Crypto::Bcrypt::Password#matches?(string) reads pretty good.

[oprypin]

Please remove == here

[oprypin]

Just gotta make sure that password == 1.23 will not compile
because normally if there is no such method it will just always return false...
and old code would keep "working"

[Sija]

7 hours ago?! GH is getting moar _fun_ every day... 🤦‍♂

[j8r]

If we go with #matches?, better to use #match? or #match. This methods already exist for others objects, better to choose this ones for consistency.

• File.match? (returns a Bool)
• Regex#match
• String#match

Crypto::Bcrypt::Password#match?(password : String) : Bool is good idea.

[Sija]

@j8r You're comparing apples to oranges. String/Regex#match is an action, matches? here is question, so the proper form would be a 3rd person (see Enumerable#includes? for instance).

[bew]

I don't like match because usually match is used to match something against a pattern, and a password is definitely not a pattern.

I'd go with is_password? or verify (or verify?)

[straight-shoota]

@bew It is actually matching the password against the stored hash by hashing it itself. So #matches? would conceptually work. But I'd also prefer #verify. That's also the term used in the methods description.

[asterite]

Just gotta make sure that password == 1.23 will not compile

I was going to say that this is kind of impossible to do, but no:

class Foo
  def ==(other : Foo)
    true
  end

  def ==(other)
    {% raise "can't compare!" %}
  end
end

foo = Foo.new
foo == 1

:-)

So I guess we can deprecated == in a first pass, then indefinitely make it a compile-error to compare it to a string.

[j8r]

It makes no sense to have sometimes 3rd person and sometimes not.
Like regex, it can be Crypto::Bcrypt::Password#match, with eventually Crypto::Bcrypt::Password#=~.
Edit: verify is nevertheless better I think, more explicit.

[Sija]

@j8r this _sometimes_ is quite specific - String/Regex#match returns MatchData, whereas ? methods are returning Bool.

[j8r]

or are nillable @Sija , there is no proper rule written anywhere for that. ([]?, to_i? etc)

[Sija]

@j8r yes, they're nillable since they return a value, not an answer (Bool).

[Sija]

@straight-shoota verify sounds good, yet since it returns Bool I'd call it verify?.

[RX14]

I don't mind the overridden ==, I'd just make it commutative

[asterite]

The problem with == is that you can basically pass anything, an int, a float, a bool, to == and it will compile, but it won't give the correct result. That makes it not type safe. And here we aren't really comparing stuff, we want to know if a password matches an encoded version of it, which isn't exactly "equality".

[straight-shoota]

The concept of a password matching an encoded version would probably be better encoded in the =~ operator. But it's not type safe either and commutativity isn't really necessary anyway.

So, using a dedicated method is IMO the best solution.
And I'd go with #verify for that.

@Sija The ? suffix is not used to indicate a Bool return type, but specifically a predicate method. #verify is more an action than a predicate. If we wanted to use a predicate, it would be #verifies?. But I think an action term fits better here.

[ysbaddaden]

Let's go with #verify? with or without the question mark.