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

Getting rid of delta transform and suggestion #3

Open
shredding opened this issue Dec 29, 2013 · 2 comments
Open

Getting rid of delta transform and suggestion #3

shredding opened this issue Dec 29, 2013 · 2 comments

Comments

@shredding
Copy link

I'm using your class as base for one of my projects (thanks!) and since i did not get my head around your delta_transform method I ended up like this, maybe that's easier?

def straight_flush?
    flush? and straight?
end

def straight?
        by_face

        # I named my version cards, it's the array of cards
        temp_cards = cards.dup

        if temp_cards[0].face == Card::FACE_VALUES['A'] and temp_cards[1].face != Card::FACE_VALUES['K']
          # it's not an ace high straight, so we move the ace to the end
          # to test for a low straight
          ace = temp_cards.shift
          temp_cards << Card.new("L#{Card::SUITS[ace.suit]}")
        end

        face_value = temp_cards[0].face

        temp_cards.each do |c|
            return false if face_value != c.face
          face_value -= 1
        end
        true
end

I as well added a by_groups func:

def by_groups
        # technically, we treat single cards as single group in this loop
        pairs = []

        until @cards.size == 0
          reference = @cards[0]
          pair, @cards = @cards.partition { |c| c.face == reference.face}
          pairs.push(pair)
        end
        pairs.sort_by! { |a| [a.size, a[0].face] }.reverse!
        pairs.each { |pair| pair.each { |c| @cards << c }}
        self
      end

It's needed for what I do, but maybe of interest here as well? I could form a pull request if you're interested at all :)

@robolson
Copy link
Owner

robolson commented Feb 9, 2014

Hi @shredding. I agree that the delta_transform method is crazy confusing although I documented it much better in late December to make it more understandable (4c2402b).

I'm sure it could be changed, but the straight? method as you've written it doesn't match the method contract of returning an array in the format [[5, high_card], arranged_hand].

I think you're version would be slower because it creates more objects. The current versions of straight and delta_transform use regular expressions which I think are faster.

However, yours is more approachable. If you could get it to pass the test suite and test/integration/test_a_million_hands.rb (see comment at top of file for how to run). Then I think I would put it in.

I looked at by_groups quickly but I do not understand what it is doing. You would need to post examples.

@shredding
Copy link
Author

by_groups is sorting a hand like A79A9 to AA997.

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