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)

• ChaiNunes: Oh my goodness, that's why it looked so bad! I knew I was missing something. Thanks!

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)

• ChaiNunes: Thanks for the non-OOP answer; I find it great, but I'm planning on sticking with a class.