Dice-rolling Python script

Question

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() 

Answer 1

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) 

Answer 2

@Daniel rightfully noted:

Your class is not a class, self is totally missing

I am going to question the basis of your design, does a class represent the best way to program a Dice rolling script?

Even if you like the idea of a class, think about about the separation of concerns, why should a Dice know how to save its result to a file?

My implementation of the script just uses some functions, and i feel like this is a big simplification, remember OOP, as any programming paradigm, is not the final perfect solution to all design problems:

import random import os def roll(sides): return random.randint(1, sides) def roll_many(sides, times): return (roll(sides) for time in range(times)) def save_list_to_file(list_, directory=''): with open(os.path.join(directory, 'savedResult.txt'), 'a') as txt: txt.write('%s\n' % ', '.join(map(str, list_))) def save_number_to_file(n, directory=''): with open(os.path.join(directory, 'savedTotal.txt'), 'a') as txt: txt.write('%d\n' % n)