Red implementation of Rock, Scissors, Paper

Question

I have implemented the Rosetta Code Rock Paper Scissors game in Red, but I'm sure that there are better ways to do every thing that I did. I would appreciate any feedback on this code, which does work according to spec:

Implement the classic children's game Rock-Paper-Scissors, as well as a simple predictive AI (artificial intelligence) player.

Rock Paper Scissors is a two player game.

Each player chooses one of rock, paper or scissors, without knowing the other player's choice.

The winner is decided by a set of rules:

• Rock beats scissors
• Scissors beat paper
• Paper beats rock

If both players choose the same thing, there is no winner for that round.

For this task, the computer will be one of the players.

The operator will select Rock, Paper or Scissors and the computer will keep a record of the choice frequency, and use that information to make a weighted random choice in an attempt to defeat its opponent.

Red [
 Problem: %http://www.rosettacode.org/wiki/Rock-paper-scissors
 Code: %https://github.com/metaperl/red-rosetta/blob/master/rock-paper-scissors.red
]
help1: %https://stackoverflow.com/questions/54272942/how-to-find-the-first-element-of-a-block-of-strings-whose-first-character-matche
help2: %https://stackoverflow.com/questions/54272956/how-to-increment-element-of-block-after-found-element
help3: %https://stackoverflow.com/questions/54273057/two-dimensional-dispatch-table-with-templated-response
help4: %https://stackoverflow.com/questions/54273161/in-red-how-do-i-search-through-a-block-for-a-string-matching-a-pattern/54275072#54275072
games-played: 0
weapons: ["rock" "scissors" "paper"]
matching-weapon: func [abbrev][
 foreach weapon weapons [
 if (first weapon) = first abbrev [
 return weapon
 ]
 ]
]
player-choices: ["rock" 0 "scissors" 0 "paper" 0 ]
player-choice-tally: func [choice][player-choices/(choice): player-choices/(choice) + 1]
player-choice: "x"
valid-choice: func [c][find "rpsq" c]
player-wins: [
 ["rock" "scissors"] "breaks"
 ["paper" "rock"] "covers"
 ["scissors" "paper"] "cut"
]
player-wins?: function [player1 player2] [
 game: reduce [player1 player2]
 winning: player-wins/(game)
]
report-win: func [player1 player2][rejoin [player1 " " (player-wins? player1 player2) " " player2]]
draw: func [player computer][player = computer]
update-stats: func [player-choice][
 player-choice-tally player-choice
 games-played: games-played + 1
]
make-computer-choice: func [][
 either games-played >= 3 [
 tmp: random games-played
 tally: select "rock" player-choices
 either tmp <= tally [return "rock"][
 tally: tally + select "scissors" player-choices
 either tmp <= tally [return "scissors"][
 return "paper"
 ]
 ]
 ][random/only weapons]
]
while [not player-choice = "q"][
 player-choice: ask "(r)ock, (s)cissors, (p)aper or (q)uit? "
 either (player-choice = "q") [][
 if (valid-choice player-choice) [
 computer-choice: random/only weapons
 player-choice: matching-weapon player-choice
 update-stats player-choice
 print rejoin ["Player choice: " player-choice "tally" player-choices "Computer choice:" computer-choice]
 either draw player-choice computer-choice [print "Draw"][
 tmp: player-wins? player-choice computer-choice
 print either tmp [rejoin ["Player wins: " report-win player-choice computer-choice]]
 [rejoin ["Computer wins: " report-win computer-choice player-choice]]
 ]
 ]
 ]
]

• Mast: This looks like a really interesting language. Welcome to Code Review and well done on your first question. Should anything be unclear about how we handle questions (Code Review works a bit differently from Stack Overflow), most of it is explained here. Feel free to ask if anything is unclear.

Answer 1

Looks an interesting first project! Some thoughts:

• The URL! type does not need a % prefix (they would be interpreted as FILE! and would behave incorrectly were you to operate upon them).

• In Red, you can use the WORD! type in place of strings. You can use a LIT-WORD! to get a word without evaluating it. Takes a little getting used to, but is more efficient and expressive:

  weapons: [rock paper scissors]
  player-choices: [rock 0 scissors 0 paper 0]
  player-wins: [
   [rock scissors] "breaks"
   [paper rock] "covers"
   [scissors paper] "cut"
  ]
  select player-choices 'rock
  

• Instead of VALID-CHOICE, you could use SWITCH to drive things:

  until [
   switch player-choice: ask "(r)ock, (s)cissors, (p)aper or (q)uit? " [
   "q" [true]
   "r" "s" "p" [
   play-round player-choice
   false
   ]
   ]
  ]
  

• You can use CASE in place of nested EITHER statements—breaks the choices out a bit:

  make-computer-choice: func [
   /local tmp tally
  ][
   case [
   games-played < 3 [
   random/only weapons
   ]
   (
   tmp: random games-played
   tmp <= tally: select player-choices 'rock
   ) [
   'rock
   ]
   tmp <= tally: tally + select player-choices 'scissors [
   'scissors
   ]
   true [ ; or else
   'paper
   ]
   ]
  ]
  

• If you want to somewhat keep the game logic separate from the interface, I'd recommend a play-round function called in the same fashion as above. play-round could return false (continues) or true (ends loop when someone wins a certain amount of games).

• I notice you define the function make-computer-choice but don't invoke it.

Answer 2

Your code seemingly follows official style guide, so that's a plus. Not all of it is properly formatted though.

In terms of implementation: textual interface is tightly coupled with game logic, which makes refactoring and code support particularly hard. I would suggest to keep all functionally related modules separately and consider usage of reactive programming - all of it will make your code more general and declarative.

---

Now, the main points:

1. Red has a literal url! datatype, but for some reason you are using file!... and it certainly doesn't mean exactly what you want it to:

>> [%http://www.rosettacode.org/wiki/Rock-paper-scissors]
== [%http :/ /www.rosettacode.org /wiki /Rock-paper-scissors]

That's one file!, one get-word! and 3 refinement!s. Instead you can just write:

>> type? http://www.rosettacode.org/wiki/Rock-paper-scissors
== url!

2. Using help* words as indices is a code smell. Even if you really need to keep these URLs as values, you should put them in a block and index them by their positions. However, I would suggest to omit cluttering source code with various cross-references. It belongs either to readme file or script header.

3. player-choice: "x" - another code smell. Red has a none value, typically used to represent the "absence" of something. You should use that instead of an arbitrarily selected string.

4. Redefeniton of pre-defined words. Your draw: ... overwrites Draw dialect function. To avoid that, either pick a different name or use contexts for encapsulation. draw definition itself is just an alias for equal? - there's no need to reinvent the wheel.

5. Zero argument function func [][...] has an idiomatic does [...] form.

6. either condition [][...] -> unless condition [...].

7. Redundant usages of rejoin. print already reduces its argument and preserves spaces between elements.

8. Redundant usages of parenthesis. It is preferred to omit them entirely.

9. not x = y -> x <> y.

10. Naming could be better. As a rule of thumb, try to pick 5-8 letter words, and avoid verbosity. Function that return boolean values should be posfixed with ? mark.

11. You say that your code works according to a spec. However, implementation of weighted random choice contains two related bugs: in particular, tally: select "rock" player-choices and select "scissors" player-choices - this expressions always return none, because you mixed up the order of arguments. Because of that, only random/only branch is picked at any time. Moreso, make-computer-choice itself is never used in the main loop.

12. Poor input checks - you consider only the first input character, but don't impose any restrictions on input length. What happens if I input, say, "rp"? Your implementation interprets it as "rock", whereas to me it looks like a typo.

13. Refactor deeply nested conditionals using case and switch. Consider usage of dispatch tables.

With that said, take more time to learn the language, as your code clearly indicates the lack of knowledge about available datatypes and functions, common idioms and leverage points.

You certainly wrote it having a typical scripting language in mind, without paying homage to Red's key strengths (dialects, data-orientation and homoiconicity) - and that's not a bad thing per se by any means. However, if you deem your code worthy of showcasing on Rosettacode, then you should spend some time on triaging all the bugs and making it idiomatic - as of now, it doesn't strike me as anything special.