Yrurtj

I wrote a script that gives a "generalized" function of an arbitrary dice. Is this any good? How can it improve so to make it cleaner?

import random

def roll(die):

    number = random.randint(0,len(die)-1)

    b = die[number]

    return b



Die1 = [1,2,3,4]

Die2 = [1,2,3,4,5,6] #num lists



def inptchacc():

    ending_conditions = ['stop','Stop','quit','Quit']

    end = False

    inpu = input('what number would you like to add to the new face of the Die? (to end the die please type "stop or Stop or quit or Quit to finish the DIE" )')

    while end == False:

        if not(inpu in ending_conditions):

            try:

                retr = int(inpu)

                return retr

            except:

                string = 'invalid input please try again'

                inpu = input('invalid input please try again ')

        else:

            stop = 'stop'

            return stop



def deeper(IDie):

    list = 

    Adding = True

    while Adding:

        print('The Die ' + IDie + ' is currently ' + str(list) )

        toadd = inptchacc()

        if toadd != 'stop':

            list.append(toadd)

        else:

            Adding = False

    return list





def chance_overlap(Die1,Die2):

    highnumber = 1000

    counter = 0

    for n in range(highnumber):

        x = roll(Die1)

        y = roll(Die2)

        if x == y:

            counter += 1

    chance = counter/highnumber

    return chance



chance = chance_overlap(Die1,Die2)



print(chance)

Doing = True

while Doing:

    try:

        IDie1 = deeper('IDie1')

        IDie2 = deeper('IDie1')

        chance2 = chance_overlap(IDie1,IDie2)

        Doing = False

    except:

        print ('incompatible options selected returning....... ')



print(chance2) 

The right functions

def roll(die):

    number = random.randint(0,len(die)-1)

    b = die[number]

    return b

Can be:

def roll(die):

    return random.choice(die)

inptchacc()

You could instead check only the first character of the input (case insensitive), rather than expecting the whole keyword.

Your return variable stop isn't needed. Just return directly.

stop = 'stop'

return stop

Should be:

return 'stop'

Likewise elsewhere in your code.

Use __main__

Rather than putting code between functions and also at the end of the file, instead use:

if __name__ == '__main__':

See here for more details.

Since you need to roll many times in the chance_overlap function, you might want to optimize making n rolls, using random.choices (Python 3.6+):

from itertools import groupby



def roll(die, n=1):

    if n == 1:

        return random.choice(die)

    return random.choices(die, k=n)



def all_equal(iterable):

    "Returns True if all the elements are equal to each other"

    g = groupby(iterable)

    return next(g, True) and not next(g, False)



def overlap_chance(*dice, n=1000):

    rolls = [roll(die, n) for die in dice]

    equal_rolls = sum(all_equal(roll) for roll in zip(*rolls))

    return equal_rolls / n

Here I chose to include it in your roll function, which is nice because you only have on function, but you do have different return types depending on the value of k, which is not so nice. If you want to you can make it into two separate functions instead.

I made chance_overlap take a variable number of dice so it even works for more than two (and also for one, which is a bit boring).

In addition, I followed Python's official style-guide, PEP8 for variable names (lower_case).

The all_equal function is directly taken from the itertools recipes.

Make the computer count

These:

Die1 = [1,2,3,4]

Die2 = [1,2,3,4,5,6]

should be

die1 = list(range(1, 5))

die2 = list(range(1, 7))

Only compare once

This list:

ending_conditions = ['stop','Stop','quit','Quit']

has two problems. First, don't store multiple cases for one word - just store lower (or upper) case - I prefer lower, because no shouting.

Also, it shouldn't be a list. It's getting tested for membership, so make it a set. In other words,

ending_conditions = {'stop', 'quiet'}

then for use, write "not in", instead of "not x in"; i.e.

if inpu.lower() not in ending_conditions:

PEP8

Run your code through a linter. It will catch several things, including

• Don't capitalize your variables (Die1, Die2, Adding, Doing) if they're local. If they're global constants, they need to be all-uppercase. In the case of Die*, they probably shouldn't be global at all.


• Add a couple of newlines after your imports

Grammar

Write:

print('Incompatible options selected. Returning...')