digitalmars.D - A little game, Phobos and more
- bearophile (351/351) Apr 12 2013 In this post I use D and Phobos, to show some problems and to
- David (2/2) Apr 12 2013 Impressive list, when I saw the code, my first impression "looks good",
In this post I use D and Phobos, to show some problems and to
suggest some improvements.
This little D program is one of the tasks of the Rosettacode
site, it allows to play a basic Rock-Paper-Scissors game on the
console against the computer:
- - - - - - - - - - - - - - - - -
import std.stdio, std.random, std.string, std.array;
immutable order = ["rock", "paper", "scissors"];
uint[string] choiceFrequency;
immutable string[string] whatBeats;
nothrow pure static this() {
whatBeats = ["paper": "scissors",
"scissors": "rock",
"rock": "paper"];
}
string checkWinner(in string a, in string b) pure nothrow {
if (b == whatBeats[a])
return b;
else if (a == whatBeats[b])
return a;
return "";
}
string getRandomChoice() {
if (choiceFrequency.length == 0)
return order[uniform(0, $)];
const choices = choiceFrequency.keys;
return whatBeats[choices[choiceFrequency.values.dice]];
}
void main() {
writeln("Rock-paper-scissors game");
while (true) {
write("Your choice: ");
immutable humanChoice = readln.strip.toLower;
if (humanChoice.empty)
break;
if (humanChoice !in whatBeats) {
writeln("Wrong input: ", humanChoice);
continue;
}
immutable compChoice = getRandomChoice;
write("Computer picked ", compChoice, ", ");
choiceFrequency[humanChoice]++;
immutable winner = checkWinner(humanChoice, compChoice);
if (winner.empty)
writeln("nobody wins!");
else
writeln(winner, " wins!");
}
}
The program has a choiceFrequency string-count associative array
that keeps counts of the human choices. The computer chooses to
play with a biased dice to beat the most probable (zero-order)
human choice (it's easy to add a first-order frequency table that
counts the adjacent pairs of human choices to improve a little
the computer playing skills).
That program is simple, but there are several small things to
improve or just discuss about.
- - - - - - - - - - - - - - - - -
1) Currently you can't write:
static this() nothrow pure {
You have to write:
nothrow pure static this() {
http://d.puremagic.com/issues/show_bug.cgi?id=6677
- - - - - - - - - -
2) This allocates a global immutable associative array:
immutable string[string] whatBeats;
nothrow pure static this() {
whatBeats = ["paper": "scissors",
"scissors": "rock",
"rock": "paper"];
}
I am glad that since some time whatBeats is allowed to be
immutable. But what we want to write is more naturally a global
variable like:
immutable whatBeats = ["paper": "scissors",
"scissors": "rock",
"rock": "paper"];
This problem is being worked on (I think there is a patch, I
don't remember what).
- - - - - - - - - -
3) Instead of this:
string getRandomChoice() {
if (choiceFrequency.length == 0)
I'd like to use std.array.empty:
if (choiceFrequency.empty)
http://d.puremagic.com/issues/show_bug.cgi?id=6409
- - - - - - - - - -
4) Choosing a single item randomly from a sequence is a very
common operation, so instead of this:
return order[uniform(0, $)];
A more natural function to use is "choice" as in Python:
return order.choice;
http://d.puremagic.com/issues/show_bug.cgi?id=4851
- - - - - - - - - -
5) This not efficient, and I'd like something shorter and faster:
const choices = choiceFrequency.keys;
return whatBeats[choices[choiceFrequency.values.dice]];
But let's skip this for now.
- - - - - - - - - -
6) I don't like to write strings more than once, because they are
magic constants:
immutable order = ["rock", "paper", "scissors"];
immutable string[string] whatBeats;
nothrow pure static this() {
whatBeats = ["paper": "scissors",
"scissors": "rock",
"rock": "paper"];
}
With a little function you don't need the associative array:
immutable order = ["rock", "paper", "scissors"];
string whatBeats(in string a) pure /*nothrow*/
in {
assert(order.canFind(a));
} body {
return order[(order.countUntil(a) + 1) % order.length];
}
Unfortunately whatBeats() can't be nothrow (why is countUntil on
the array not nothrow?), so checkWinner() can't be nothrow.
I am not so sure whatBeats() is better than the whatBeats[]
associative array. The function is more DRY, but it contains code
that needs to be unittested, while (in theory) that little
associative array just need one inspection to tell if it's
correct. In the end I have chosen the little function.
- - - - - - - - - -
7) It's generally a bad idea to repeat a raw type like "string"
everywhere, it's better to define an alias and use it instead:
alias Choice = string;
Even better is to use a strong typedef:
alias Choice = Typedef!string;
Thankfully now this Typedef works with limited changes to the
program, like:
immutable humanChoice = readln.strip.toLower.Choice;
- - - - - - - - - -
8) The function checkWinner() returns an empty string. Here a
safer program should use something like a Nullable!Choice:
Nullable!Choice checkWinner(in Choice a, in Choice b)
pure /*nothrow*/ {
if (b == whatBeats(a))
return typeof(return)(b);
else if (a == whatBeats(b))
return typeof(return)(a);
return typeof(return).init;
}
But it doesn't work, because a and b are constant (and scope),
while here the Nullable return type expects a not constant Choice.
If I try to define a Choice with a constant string to try to
solve the problem:
alias Choice = Typedef!(const string);
I receive an ICE that I have just filed in Bugzilla:
http://d.puremagic.com/issues/show_bug.cgi?id=9923
See also:
http://d.puremagic.com/issues/show_bug.cgi?id=9166
- - - - - - - - - -
9) Here I suggest helper functions to improve that code a little:
http://d.puremagic.com/issues/show_bug.cgi?id=9637
Nullable!Choice checkWinner(in Choice a, in Choice b)
pure /*nothrow*/ {
if (b == whatBeats(a))
return nullable(b);
else if (a == whatBeats(b))
return nullable(a);
return typeof(return).init;
}
But that should be done carefully, because here nullable() is
taking a Const(Choice) while the funciton returns a
Nullable!Choice that is not const, so a naive helper function
like this can't work (there are similar troubles with Tuple):
auto nullable(T)(T x) {
return Nullable!T(x);
}
Regarding this:
return typeof(return).init;
See also (but it's not very related):
http://d.puremagic.com/issues/show_bug.cgi?id=9636
- - - - - - - - - -
10) In a such small program it's probably acceptable to use
strings. But if I am using a staticall typed language and the
program gets larger I prefer to use enums, that give more static
safety. This is an obvious way to define a Choice:
enum Choice { rock, paper, scissors }
But after this change there are several small problems to solve,
listed below. Things get a bit messy.
- - - - - - - - - -
11) In D the index of arrays is just an integer, it can't be a
range of chars (like 'a'..'z') or an enum, like in Ada,
ObjectPascal, etc.
So this still defines an associative array, that for this program
is OK, but for a larger program with tens or hundreds of enums is
a waste of space and performance:
uint[Choice] choiceFrequency;
This is not wasteful (beside a bit of template bloat):
uint[EnumMembers!Choice.length] choiceFrequency;
(This code is uses EnumMembers because a D enum doesn't know how
many members it has:
http://d.puremagic.com/issues/show_bug.cgi?id=4997 ).
But that's about as statically safe as the associative array of
strings, because if Choice is not contigous, or if its members
have sparse values, it will not work:
enum Choice { rock, paper = 5, scissors = 10 }
It's not too much hard to write a static assert that calls a
compile-time function that varifies the enum to be contigous, but
I think this is overengeneering for this program.
- - - - - - - - - -
12) Given the Choice enum, how do you write a safe whatBeats()
function? A simple way to do it is to convert the enum into an
array:
Choice whatBeats(in Choice ch) pure /*nothrow*/ {
static immutable order = [EnumMembers!Choice];
return order[(order.countUntil(ch) + 1) % $];
}
In theory the items of an enum have an order. In theory you
should be able to iterate on the members (in D you can do it
converting the EnumMembers in an eager array or using a static
foreach), find the precedent and the successor of a member, and
tell if a member is the last or first one of an enum. In Ada
there are little accessors to do this, but in D you can write
little functions as FirstMember, LastMember, predMember,
nextMember:
http://d.puremagic.com/issues/show_bug.cgi?id=9924
With those functions you can write:
Choice whatBeats(in Choice ch) /*pure nothrow*/ {
if (ch == LastMember!Choice)
return FirstMember!Choice;
else
return nextMember(ch);
}
I have not used UCSF to avoid chashing with enum members. That's
why lot of time ago in Issue 4997 I have suggested to add a
single "meta" or "meta__" field to enums, and put all the enum
attributes inside (after) it. So only the meta__ enum field name
can clash.
In Issue 9924 Walter argues against little functions like
LastMember, etc.
- - - - - - - - - -
13) Thanks to a recent Phobos improvement it's now possible to
get a uniformly random enum. So getRandomChoice() is easy:
Choice getRandomChoice() /*nothrow*/ {
if (choiceFrequency[].reduce!q{a + b} == 0)
return uniform!Choice;
return [EnumMembers!Choice][choiceFrequency.dice].whatBeats;
}
And sum() will help:
http://d.puremagic.com/issues/show_bug.cgi?id=4725
http://d.puremagic.com/issues/show_bug.cgi?id=7934
https://github.com/D-Programming-Language/phobos/pull/1205
Choice getRandomChoice() /*nothrow*/ {
if (choiceFrequency.sum)
return uniform!Choice;
return [EnumMembers!Choice][choiceFrequency.dice].whatBeats;
}
- - - - - - - - - -
14) The main uses another Phobos improvement, that allows to
convert strings to enums:
void main() {
writeln("Rock-Paper-Scissors Game");
while (true) {
write("Your choice (empty to end game): ");
immutable humanChoiceStr = readln.strip.toLower;
if (humanChoiceStr.empty)
break;
Choice humanChoice;
try {
humanChoice = humanChoiceStr.to!Choice;
} catch (ConvException e) {
writeln("Wrong input: ", humanChoiceStr);
continue;
}
...
}
}
But here I'd like to use a maybeTo!() (it's just a wrapper that
calls to!() inside):
http://d.puremagic.com/issues/show_bug.cgi?id=6840
maybeTo is similar to to!(), but instead of using exceptions it
returns a Nullable. (In functional languages this is often the
preferred design, because pattern matching makes this design very
safe).
With it the code gets simpler:
Choice humanChoice = humanChoiceStr.maybeTo!Choice;
if (humanChoice.isNull) {
writeln("Wrong input: ", humanChoiceStr);
continue;
}
- - - - - - - - - -
15) You can't use this:
writeln(winner, " wins!");
As you see from this little test program:
import std.stdio: writeln;
import std.typecons: Nullable;
enum Foo { red, green, blue }
void main() {
Nullable!Foo(Foo.green).writeln;
}
That prints:
Nullable!(Foo)(green, false)
Is this a good printing for nullables? In Bugzilla I have
suggested something different.
So you need to use:
writeln(winner.get, " wins!");
- - - - - - - - - -
16) One possible whole program with enums (watch for bugs):
import std.stdio, std.random, std.string, std.array, std.typecons,
std.traits, std.conv, std.algorithm;
enum Choice { rock, paper, scissors }
immutable order = [EnumMembers!Choice];
uint[order.length] choiceFrequency;
Choice whatBeats(in Choice ch) pure /*nothrow*/ {
return order[(order.countUntil(ch) + 1) % $];
}
Nullable!Choice checkWinner(in Choice a, in Choice b)
pure /*nothrow*/ {
alias TResult = typeof(return);
if (b == whatBeats(a))
return TResult(b);
else if (a == whatBeats(b))
return TResult(a);
return TResult.init;
}
Choice getRandomChoice() /*nothrow*/ {
if (choiceFrequency[].reduce!q{a + b} == 0)
return uniform!Choice;
return order[choiceFrequency.dice].whatBeats;
}
void main() {
writeln("Rock-Paper-Scissors Game");
while (true) {
write("Your choice (empty to end game): ");
immutable humanChoiceStr = readln.strip.toLower;
if (humanChoiceStr.empty)
break;
Choice humanChoice;
try {
humanChoice = humanChoiceStr.to!Choice;
} catch (ConvException e) {
writeln("Wrong input: ", humanChoiceStr);
continue;
}
immutable compChoice = getRandomChoice;
write("Computer picked ", compChoice, ", ");
// Don't register the player choice until after
// the computer has made its choice.
choiceFrequency[humanChoice]++;
immutable winner = checkWinner(humanChoice, compChoice);
if (winner.isNull)
writeln("Nobody wins!");
else
writeln(winner.get, " wins!");
}
}
Bye,
bearophile
Apr 12 2013
David <d dav1d.de> Impressive list, when I saw the code, my first impression "looks good", thanks for opening my eyes!
Apr 12 2013