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

Add new practice exercise yacht #693

Merged
merged 4 commits into from
May 21, 2024
Merged

Add new practice exercise yacht #693

merged 4 commits into from
May 21, 2024

Conversation

jiegillet
Copy link
Contributor

This one is week 22 of 48in24.

Overall a pretty nice one, my way of solving it included creating some kind of group function, which made me realize that we don't have a recursion concept (#692).

}


score : List Int -> Category -> Int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how important this is, but I found this hard to understand. There are some relationships. / Requirements between the functions that are implicit, and aren't in the type system, it really even as comments most of the time. It fellas like there is a better solution out there, although I haven't tried to find it. I think full house is the most obvious example, where it requires the 3 dice to be the first part of the tuple, and the 2 dice to be the second part

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, it's not massively important that my solution be easy to understand, it's merely a proof that it can be solved, but it would be nice to try anyway.

The key of my solution is groupDice which takes 5 numbers and groups them into groups in order of descending count. For example groupDice [5, 1, 5, 1, 5] == [{dice = 5, count = 3}, {dice = 1, count = 2}], 5 is there 3 times and 1 is there 2 times. 5 is first since it's the most frequent. When I check for a full house, I pattern match on lists of 2 groups and check that the counts are 3 and 2, and that guarantees a success.
Each category can use this grouping in different way, for example for a straight, you need 5 different dice values (pattern match on lists of length 5) and only a specific dice missing (1 or 6) which can be easily checked if the groups are ordered properly.

What would you suggest I do to make it clearer? Comments? Something more?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, so I think the requirements / invariants that score requires of groupDice are as follows

  • Dice with a zero count aren't present
  • List is sorted in order of descending count, then by dice value
  • Dice are not repeated (you might reasonably assume this though)
  • Dice numbers are from 1 - 6 (you would definitely assume this)

A lot of this is hard to represent in the type system. You could maybe use a Set to guarantee uniqueness, but it might cause other problems. I think in a real codebase I would represent the List DiceGroup as an opaque type, with an intent revealing name, so that it could uphold the invariants, but I think that is probably overkill for this.

Maybe the easiest thing to do is move the requirements in to score for now. It could easily sort the list itself, and maybe remove any zero counts (which there won't be any of, but it still makes it clear what is happening. You could even check for 1 - 6 and repeats as well to be really defensive, and be able to handle any List DiceGroup, which is what the type system is saying it can do.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it strange to do defensive programming on an algorithm for things that cannot happen.
I think that an implementation based folding with a Dict Dice Count accumulator might exhibit the properties you want, I'll try that first.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it, and it turned out quite clean. I didn't bother with defining a type so that I could pattern match more easily. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to go with it, it is a bit more obvious what score requires of group now.

@jiegillet jiegillet merged commit 2b7bd6a into main May 21, 2024
6 checks passed
@jiegillet jiegillet deleted the jie-yacht branch May 21, 2024 23:33
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

Successfully merging this pull request may close these issues.

2 participants