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

Dice-rolling Python script

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

Problem

I completed writing a dice roll script in Python but I thought it looked too messy. Is there anything I should change here?

import random, os

class Dice:
    result = []
    total = 0

    def __roll_(sides=1):
        return random.randint(1, sides)

    def roll(sides=1, times=1):
        for time in range(0, times):
            Dice.result.append(Dice.__roll_(sides))
            Dice.result = Dice.result[len(Dice.result) - times:len(Dice.result)]
        Dice.sumResult()
        return Dice.result

    def sumResult():
        Dice.total = 0
        for num in range(0, len(Dice.result)):
            Dice.total += Dice.result[num]
        return Dice.total

    def saveResult(directory=''):
        if directory == '':
            savetxt = open('savedResult.txt', 'a+')
        else:
            savetxt = open(os.path.join(directory, 'savedResult.txt'), 'a+')
        savetxt.write(str(Dice.result) + '\n')
        savetxt.close()

    def saveTotal(directory=''):
        if directory == '':
            savetxt = open('savedTotal.txt', 'a+')
        else:
            savetxt = open(os.path.join(directory, 'savedTotal.txt'), 'a+')
        savetxt.write(str(Dice.total) + '\n')
        savetxt.close()

Solution

Your class is not a class, self is totally missing. You have to rewrite the whole thing.
Internal methods start with one single underscore _roll.
You can access lists from the end with negative indices, len in unnesseccary.
Never change the internal state of a instance and return a value. Do the one or the other.
You can join with empty strings, if is unneccessary.
Open files with the with-statement. Never use the string representation of python objects like lists or dicts for other purposes than debugging.
Remember the naming conventions in PEP-8.

import random
import os

class Dice:
    def __init__(self, sides=1):
        self.sides = sides
        self.result = []
        self.total = 0

    def _roll(self):
        return random.randint(1, self.sides)

    def roll(self, times=1):
        self.result[:] = [self._roll() for time in range(times)]
        self.sum_result()

    def sum_result(self):
        self.total = sum(self.result)

    def save_result(self, directory=''):
        with open(os.path.join(directory, 'savedResult.txt'), 'a') as txt:
            txt.write('%s\n' % ', '.join(map(str, self.result)))

    def save_total(directory=''):
        with open(os.path.join(directory, 'savedTotal.txt'), 'a') as txt:
            txt.write('%d\n' % self.total)

Code Snippets

import random
import os

class Dice:
    def __init__(self, sides=1):
        self.sides = sides
        self.result = []
        self.total = 0

    def _roll(self):
        return random.randint(1, self.sides)

    def roll(self, times=1):
        self.result[:] = [self._roll() for time in range(times)]
        self.sum_result()

    def sum_result(self):
        self.total = sum(self.result)

    def save_result(self, directory=''):
        with open(os.path.join(directory, 'savedResult.txt'), 'a') as txt:
            txt.write('%s\n' % ', '.join(map(str, self.result)))

    def save_total(directory=''):
        with open(os.path.join(directory, 'savedTotal.txt'), 'a') as txt:
            txt.write('%d\n' % self.total)

Context

StackExchange Code Review Q#115700, answer score: 4

Revisions (0)

No revisions yet.