Dice-rolling Python script

Question 1

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

Question 2

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 randomimport osclass 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)

Question 3

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

Question 4

@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 randomimport osdef 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)

Question 5

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

Daniel 4183 silver badges5 bronze badges · Accepted Answer · 2016-01-03 08:04:22Z

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 randomimport osclass 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)

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

Movatterモバイル変換

Stack Exchange Network

Dice-rolling Python script

2 Answers2

You mustlog in to answer this question.

Linked

Related

Hot Network Questions

Subscribe to RSS