Integrate `014/training` into `solution`

ga-wdi-boston / ruby-enumerable-comparable

Other

1 stars 139 forks source link

Integrate `014/training` into `solution` #22

Open jrhorn424 opened 7 years ago

jrhorn424 commented 7 years ago

These commits:

546df355563619f6458d946639bb14d364c4c826
b44111bfdb2e9fa85cf6d7d446c157877a507e59
01fc422f56c3b032f59b1db04d6703e2162e72c3
ca499745ffb8db2e3c29e7363852ab0c837e82b4
7085bb35531d269cb5f77827774150b74b08477d

gaand commented 7 years ago

For the last commit above, 7085bb35531d269cb5f77827774150b74b08477d, I would prefer a guard clause on the solution branch which short circuits the calculations.

Also, compare rank first.

Also, also, I think the referenced solution may be incorrect in most card games.

  def <=>(other)
    rank_comparison = RANKS.index(rank) <=> RANKS.index(other.rank)
    return rank_comparison unless rank_comparison.zero?
    SUITS.index(suit) <=> SUITS.index(other.suit)
  end

jrhorn424 commented 7 years ago

Also, also, I think the referenced solution may be incorrect in most card games.

@gaand That's true. I seem to remember that it was the way a developer suggested doing, maybe back in 012? The solution you've pasted above is functionally equivalent, with the comparisons reversed from the current solution (checking I'm understanding your intent here). I like it.

gaand commented 7 years ago

No. The solution above compares rank first. Only then does it compare suit. The commit referenced checks suit first, then rank. So, a two of spades is "bigger" than and ace of clubs.