Bug#682454: RFS: cows-and-bulls/1.0-1 [ITP] - Words-based cows and bulls guessing game
Hi Jakub,
Thank you for reviewing this.
On Tue, Jul 31, 2012 at 2:28 PM, Jakub Wilk <jwilk@debian.org> wrote:
> (I don't intend to sponsor this package.)
> It doesn't work in a minimal environment:
> | $ cows-and-bulls
> | Unable to open the dictionary file
The default dictionary file used by this game is /usr/share/dict/words
which is installed by the package wamerican. I don't know if or not
the file will be present in non-English locale environments and even
if it is there, it might contain non-ascii characters in words.
> Perhaps some dependencies are missing?
Should I list 'wamerican' package as a dependency?
> The game segfaults if stdin ends prematurely:
> | $ cows-and-bulls </dev/null
> | Enter the length of the word to be used to play(3-8):Enter a valid length
> to be used (3-8)
> | Segmentation fault
>
> Or it falls into infinite loop:
> | $ echo 4 | cows-and-bulls
> | Enter the length of the word to be used to play(3-8):Guess the word:The
> guessed word should be 4 characters long
> | Guess the word:The guessed word should be 4 characters long
> | Guess the word:The guessed word should be 4 characters long
> | Guess the word:The guessed word should be 4 characters long
> | Guess the word:The guessed word should be 4 characters long
> | Guess the word:The guessed word should be 4 characters long
> | Guess the word:The guessed word should be 4 characters long
> [snip - ad infinitum...]
>
> If I enter invalid length, the game continues:
> | $ cows-and-bulls
> | Enter the length of the word to be used to play(3-8):1
> | Enter a valid length to be used (3-8)
> | Guess the word:what?
> | The guessed word should be 1 characters long
>
> It appears to me that you count bytes, not characters. This is incorrect, as
> some words in /usr/share/dict/words are non-ASCII. Perhaps you should filter
> them out, to avoid surprises.
There is a bug in the code due to a missing 'continue' statement. This
can be fixed.
>
> Now looking at the source:
>
>> srand(time(0));
>
>
> You should normally call srand only once, at the start of the program.
>
>> int n = (rand() % (words_of_length_n.size()-1)) + 1;
>> return words_of_length_n[n];
>
>
> As far as I can see, this exclude the very first (0th) word. Why?
>
>> cout<<cows<<" Cows and "<<bulls<<" bulls"<<endl;
>
>
> Is there a reason "Cows" is capitalized?
>
>> cout<<"Enter the length of the word to be used to
>> play("<<MIN_WORD_LENGTH
>
>
> Missing space before "(" and after ":".
>
>> cout<<"Guess the word:";
>
>
> Missing space after ":".
>
>> cout<<"Do you want to play again(y/n)?:";
>
>
> Ditto.
>
>
> All in all, I found it relatively low quality, especially if you take into
It is really buggy and not well written, I agree.
> account that it's a game that one could be implement in 5 minutes in a
> scripting language. Sorry.
I wanted to write this in Python which I am most comfortable with. But
wanted to use this as an opportunity to learn and use C++ and ended up
with so many issues, which I will try to fix. Me thinks using Python,
this can be done with a lot more elegant and cleaner code, so will
consider writing this in python as well. Or maybe a GUI
implementation. But since I don't have much experience with GUI
development, it could be buggy.
Thank you for your valuable feedback.
Reply to: