HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Rock, Paper, Whatever - A small commandline game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
papercommandlinegamesmallrockwhatever

Problem

My rational for this program, was to learn more of Python's standard library, I didn't actually know cmd was even in it until I started this program. It's also the first community challenge, and I have some catching up to do.

This is an overly engineered version of the famous Rock Paper Scissors game. By default it's the exact same, but allows for people to define their own weapons, such as Rock Paper Scissors Lizard Spock, or the less liked Rock Paper Scissors Chainsaw. To do this I used configparser for an INI like input format. The sections are the winners, where the keys that follow are the losers. I also allow keys to be passed values which are the winning action, defaulting to 'beats'. Here is an example of a configuration file used for Rock Paper Scissors Lizard Spock (saved as rpsls, and used in the example):

[rock]
scissors: crushes
lizard: crushes

[paper]
rock: covers
spock: disproves

[scissors]
paper: cuts
lizard: decapitates

[lizard]
paper: eats
spock: poisons

[spock]
rock: vaporizes
scissors: smashes


To simplify the usage of this program I made some simple directed Graph and Node classes that inherit from dict. Which I think follow SOLID.

I also used cmd to create a simple command line interface. It didn't need to be anything fancy, and so it seemed like the best, and simplest, solution.

This also only works in Python 3.6, as I used f-strings.

```
import cmd
import configparser
import itertools
import random

class RPSException(Exception):
pass

class DirectedGraph(dict):
class Node(dict):
__slots__ = ['name', '_graph']
def __init__(self, graph, name):
self.name = name
self._graph = graph

def add_edge(self, node, edge):
self[node.name] = edge

def adjacent(self, x, y):
return x in self[y] or y in self[x]

def add_node(self, *args, **kwargs):
node = self.Node(self, *args, **kwargs)
self[node.name] = node

def add_edge(self, node

Solution

First things first:

1.PEP8

Please run any linter, since your code violates PEP8.

-
Tripple double quotes should be used in doctrings

-
Linebreaks(backslashes) are not recommended, preferred way is parenthesis

  1. Nested classes



Speaking about your graph class structure. I try to avoid inner(nested) classes since they cause more troubles(e.g pickling, testing, less readable for me) rather than helping you defining a scope.

  1. Code improvements



class RPS(cmd.Cmd):
    ...
    cfg = configparser.ConfigParser(allow_no_value=True)
    cfg.read_string('''\
    [rock]
    scissors: crushes

    [paper]
    rock: covers

    [scissors]
    paper: cuts
    ''')
    graph = parse_config(cfg)
    del cfg
    ...


I'm not really a fan of having logics within a class body definition, I think better make config a lazy property that is when not defined and called will do this trick for you.

Now here:

ai = random.choice(list(graph.keys()))
print(', '.join(self.graph.keys()))


You don't really have to call .keys() method since by default it iterates over dictionary keys. So you can just do:

print(', '.join(self.graph))


This:

def missing_edges(self):
    for keys in itertools.combinations(self.keys(), 2):
        if not self.adjacent(*keys):
            yield keys


Can be replaced by combination of filter and yield from statement.
It's also not clear why you define this as a generator, while in the only place you use it, you cast it to list.

When you have already a function I prefer using map instead of list comprehensions so in places like that:

return '\n'.join(str(node) for node in self.values())


I will go for

return '\n'.join(map(str, self.values()))

Code Snippets

class RPS(cmd.Cmd):
    ...
    cfg = configparser.ConfigParser(allow_no_value=True)
    cfg.read_string('''\
    [rock]
    scissors: crushes

    [paper]
    rock: covers

    [scissors]
    paper: cuts
    ''')
    graph = parse_config(cfg)
    del cfg
    ...
ai = random.choice(list(graph.keys()))
print(', '.join(self.graph.keys()))
print(', '.join(self.graph))
def missing_edges(self):
    for keys in itertools.combinations(self.keys(), 2):
        if not self.adjacent(*keys):
            yield keys
return '\n'.join(str(node) for node in self.values())

Context

StackExchange Code Review Q#155839, answer score: 6

Revisions (0)

No revisions yet.