0

I haven't quite verified the correctness and lack of off-by-one bugs, but bear with me for a moment.

The goal here is to deal/split a deck of cards (defined as a {Card*}) into multiple Decks (which optionally take a {Card*} as a constructor argument). I want to split the cards in a round-robin fashion, like cards would actually be dealt. Here's the code I have so far:

{Deck*} split(Integer ways) {
    assert(theCards.size % ways == 0);
    {Card*} empty = {};
    value decks = LinkedList { for (i in 0:ways) empty };
    for(i -> card in entries(theCards)) {
        value deckIndex = i % ways;
        assert (exists current = decks[deckIndex]);
        decks.set(deckIndex, {card, *current});
    }
    return { for(cards in decks) Deck(cards) };
}
  1. Is this the correct/idiomatic way to split a list into multiple lists?
  2. If I wanted to not reverse all the cards (that is, append to the list instead of prepend, or reverse the iterable) how might I do that?
  3. How would I initialize the values of the decks variable lazily inside the loop?
  4. Is there any way I can get away from needing the empty variable I have?
  5. Any chance I could write this without the need for mutable data structures?

Thanks, and sorry for the multi-question (I'd have created this on codereview.stackexchange.com but I don't have the rep to create a ceylon tag there).

Max
  • 4,882
  • 2
  • 29
  • 43

2 Answers2

4

Not reversed, lazy, no mutable data structures:

alias Card => String;

{{Card*}*} deal({Card*} cards, Integer players)
        => {for (i in 0:players) cards.skipping(i).by(players)};

void run() {
    value suits = {"♦", "♣", "♥", "♠"};
    value ranks = {for (i in 2..10) i.string}.chain{"J", "Q", "K", "A"};
    value deck = {for (suit in suits) for (rank in ranks) rank + suit};
    print(deal(deck.taking(10), 2)); // { { 2♦, 4♦, 6♦, 8♦, 10♦ }, { 3♦, 5♦, 7♦, 9♦, J♦ } }
}

The laziness and immutable style comes at the cost of iterating through all of the cards for each hand. I prefer this eager solution:

{Card[]*} deal({Card*} cards, Integer players) {
    value hands = [for (i in 0:players) SequenceBuilder<Card>()];
    for (card->hand in zipEntries(cards, hands.cycled)) {
        hand.append(card);
    }
    return {for (hand in hands) hand.sequence};
}

That way, you're just iterating the deck once.


Note that Ceylon's enumerated types provide a nifty way to represent things like suits and ranks in a type-safe and object-oriented way, analogously to Java's enums:

abstract class Suit(shared actual String string)
        of diamonds | clubs | hearts | spades {}

object diamonds extends Suit("♦") {}
object clubs extends Suit("♣") {}
object hearts extends Suit("♥") {}
object spades extends Suit("♠") {}
gdejohn
  • 7,451
  • 1
  • 33
  • 49
  • Thanks, this is just what I was looking for. It's mildly clever, but not too clever. FWIW I actually am representing my suits and ranks as objects, but representing ranks as objects as opposed to what you have above feels a bit forced. I like the use of utf8 characters here, too. The use of #chain hadn't occurred to me; haven't learned/memorized the full API yet. Anyway, thanks! – Max Dec 10 '13 at 04:40
  • On second glance, this I believe also iterates over the entire input `hands` times. Still, it's a fair bit cleaner (sorry Gavin). – Max Dec 10 '13 at 06:40
  • It does, but I don't think you can get around that while maintaining laziness and immutability. – gdejohn Dec 10 '13 at 06:50
  • This is quite similar to mine except that it didn't occur to me to use `Iterable.skipping()` and `Iterable.by()`. – Gavin King Dec 10 '13 at 10:03
  • Yeah, @Max, my lazy solution works basically the same as Gavin's. In any case, I think an eager solution is more appropriate. See the edit in the middle of my answer. – gdejohn Dec 10 '13 at 11:39
  • Yeah...that's true. So, noob question, what's the postfix * syntax about? Is that in the Tour of Ceylon anywhere? – Max Dec 10 '13 at 16:05
  • @Max, do you mean `hands*.sequence`? That's the [spread attribute operator](http://ceylon-lang.org/documentation/reference/operator/spread-attribute/). It returns a sequence, the elements of which are the respective attributes of each of the elements in the receiving iterable (same thing can be done with methods). In this case, since the attribute in question is itself a sequence, the result is a sequence of sequences. Equivalently, `[for (builder in hands) builder.sequence]`. – gdejohn Dec 10 '13 at 21:59
  • Great, thanks! I knew about the spread invoke operator, but not the spread attribute operator. – Max Dec 10 '13 at 23:16
  • @Max, just reworked the eager solution. Check it out. – gdejohn Dec 10 '13 at 23:42
  • @gdejohn Looks great! I'm doubtful there's any value in an iterable-of-sequences as a return type, but it doesn't matter terribly. I like the zipEntries+iterable.cycled pattern, especially. – Max Dec 11 '13 at 18:30
  • Heh, I finally got around to running this just now...it throws a ClassCastException. I'll open a bug against the compiler. – Max Dec 12 '13 at 16:34
  • @Max, did you make any changes to the code I posted? It all works fine for me. – gdejohn Dec 13 '13 at 03:00
  • @gdejohn I was incorporating it into my own code and ended up hitting three compiler bugs (two of which causes the runtime to crash and one which causes an infinite loop) and one IDE bug along the way. I'm working on getting permission from my employer to report them. But I think the code you provided does work! Just the support code I have around it doesn't. – Max Dec 13 '13 at 17:53
  • @Max, sounds like some intense support code. Anyway, regarding the iterable of sequences returned by my eager solution, my thinking is that laziness should be the default, and code should only be eager when there's a reason for it. Of course there are valid reasons you might want to get back a sequence of sequences right away, but I can't see any of your surrounding code, so I fell back on laziness. – gdejohn Dec 14 '13 at 00:46
  • @gdejohn Oh, sure, I'm all for returning iterables -- I just meant that I feel like a sequence-of-sequences and iterable-of-sequences are probably going to be about the same, performance-wise (where the size of the iterable is small). Now, either of those vs. iterable-of-iterable I'd expect there to be a noticeable difference... – Max Dec 14 '13 at 06:34
1

How about something like:

alias Deck => {Card*};

{Deck*} split(Deck cards, Integer ways) {
    assert(ways.divides(cards.size));
    return { for i in (0:ways) { for (j->card in entries(cards)) if (j%ways==i) card } };
}

If Deck is actually a class, not very clear from your description, then this:

class Deck(shared {Card*} cards) { ... }

{Deck*} split(Deck deck, Integer ways) {
    assert(ways.divides(deck.cards.size));
    return { for i in (0:ways) Deck { for (j->card in entries(deck.cards)) if (j%ways==i) card } };
}

Note: I have not tested this.

Gavin King
  • 3,182
  • 1
  • 13
  • 11
  • Sorry, yes, Deck is a class, and theCards is a property containing {Card*}. The Deck prototype isn't exactly the same as what you have there (cards isn't shared) but it's close enough... Also that solution might work, but it looks like it iterates over all the cards `ways` times, which isn't ideal. – Max Dec 09 '13 at 22:06
  • Well, it does more iteration, but it does much less allocation. You would have to actually benchmark it to be sure about which is faster. If I had to guess, I would guess that my version is faster, but I have learned long ago not to trust my intuition on such things. – Gavin King Dec 10 '13 at 00:37
  • Where would the allocation be expensive? `{card, *current}`? Is that O(n)? – Max Dec 10 '13 at 04:41
  • Well you do `{card, *current}` inside a loop. You're basically creating a long linked list. It's not even an efficient way to create a linked list. – Gavin King Dec 10 '13 at 09:59