Arbitrary probability dice

Question

I have to make this: Each set consists of a number of equal objects that generate numbers from a finite set with an arbitrary probability distribution. I did make it, but it feels so long. How can I refactor the code to something better and clean looking? Also, don't mind certain variable names and callbacks. I've done it for satire purposes

#Assignment
import time
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(string):
    ending_conditions = ['stop','Stop','quit','Quit']
    end = False
    inpu = input(string)
    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 quantitymatrix(IDie):
    stringDie = 'how often would you like this Die to occur?'
    list = []
    Adding = True
    while Adding:
        print('the Die in question is '+ str(IDie))
        toadd = inptchacc(string)
        if toadd != 'stop':
            list.append(toadd)
        else:
            Adding = False
    return list

def deeper(IDie):
    stringDie = '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" )'
    list = []
    Adding = True
    while Adding:
        print('The Die ' + IDie + ' is currently ' + str(list) )
        toadd = inptchacc(stringDie)
        if toadd != 'stop':
            list.append(toadd)
        else:
            Adding = False
    return list

def chance_overlap(dielist,Dielistcount):
    highnumber = 100000
    counter = (len(dielist))*[0]
    chance = (len(dielist))*[0]
    for n in range(highnumber):
        dieres = len(Dielistcount)*[0]
        for dienumber in range(len(dielist)):
            for diecount in range(Dielistcount[dienumber]):
                dieres[dienumber] += roll(dielist[dienumber])
        for dienumber2 in range(len(dielist)):
            if max(dieres) == dieres[dienumber2] and dieres.count(max(dieres)) == 1:
                counter[dienumber2] += 1
    for chanceper in range(len(counter)):
        chance[chanceper] = counter[chanceper]/highnumber
        chance[chanceper] = str(chance[chanceper]) + '% for die' + str(chanceper+1)
    return chance

def suckmypp(counterq):
    string1 = 'adding the amount of the die '+ str(counterq+1)
    firstq = True
    while firstq:
        suckmypp2 = inptchacc(string1)
        if suckmypp2 != 'stop':
            firstq = False
    return suckmypp2

Dielist1 = [Die1,Die2]
diecount = [9,6]
chance = chance_overlap(Dielist1,diecount)

print(chance)
Doing = True
counter = 0

while Doing:

        Dielist2 = []
        adding = True
        while adding:

            counter += 1
            addQ = input('to stop type S and enter otherwise any characters and enter will add another die')
            if addQ != 'S':
                notdone = True
                while notdone:
                    dietoadd = deeper('Die' + str(counter))
                    if len(dietoadd) >= 1:
                        Dielist2.append(dietoadd)
                        notdone = False
                    else:
                        print('die is empty not added')
            else:
                adding = False
        quantity = True
        counterq = 0
        Qlist = []
        print(Qlist)
        print(len(Dielist2))
        while quantity:
            Qlist.append(suckmypp(counterq))
            counterq += 1
            if counterq == (len(Dielist2)):
                quantity = False

        print(Dielist2)
        print(Qlist)
        chance2 = chance_overlap(Dielist2,Qlist)
        Doing = False


print(chance2) 

Answer 1

roll()

• Your roll function implements random.choice, so its definition can be replaced with

  from random import choice as roll
  

inptchacc()

• Instead of having several words for upper-case/lower-case letters, it is more convenient to declare only lower-case versions and convert input to lower-case before comparing. Also, using operator not in is more readable than not ... in ...:

  stop_words = ['stop', 'quit']
  if inpu.lower() not in stop_words:
      # ...
  

• You do not need variable end; remove it and use a while True loop.

• You can place inpu = input() at the beginning of the while loop to avoid its duplication.
• You do not need to store value into a variable before returning it; just return an expression.
• Using a guard clause for checking stop condition will reduce nesting.
• Renaming string to prompt looks appropriate. Also, try to avoid ugly names like inpu. Use input_ or value instead.

  If your public attribute name collides with a reserved keyword, append a single trailing underscore to your attribute name. This is preferable to an abbreviation or corrupted spelling. (PEP-8)

def inptchacc(string):
    while True:
        inpu = input(string)
        if inpu in ['stop','quit']:
            return 'stop'
        try:
            return int(inpu)
        except:
            string = 'invalid input please try again'

quantitymatrix() and deeper()

• quantitymatrix() is unused.
• According to PEP-8:

  Function names should be lowercase, with words separated by underscores as necessary to improve readability. Variable names follow the same convention as function names.

• You do not need the variable Adding, just use whilte True and replace Adding = False with break
• It is bad practice to override builtin functions (list), better use list_ or lst. Actually, here name die is more appropriate since this list represents a die!
• Use print() with several arguments instead of converting list to str.

def deeper(idie):
    die = []
    while True:
        print('The Die ' + idie + ' is currently', die)
        to_add = inptchacc(
                'what number would you like to add to the new face '
                'of the Die? (to end the die type "stop" or "quit")'
                )
        if to_add == 'stop':
            break
        die.append(to_add)
    return die

chance_overlap()

• Use _ or __ name for unused variables (n, diecount).

• dieres evaluation may be done like this:

  dieres = [
      sum(roll(die) for __ in range(repeat))
      for (die, repeat) in zip(die_list, die_list_count)
  ]
  

• When updating counter it is more efficient to check if there is a winner outside of the for loop. Actually, you do not need the for loop at all:

  maximum = max(dieres)
  if dieres.count(maximum) == 1:
      winner = dieres.index(maximum)
      counter[winner] += 1
  

• You do not need the chance list; just replace last loop with list comprehension:

  return [
      str(count / highnumber) + '% for die' + str(i)
      for (i, count) in enumerate(counter, start=1)
  ]
  

So the final version of chance_overlap() is:

def chance_overlap(die_list, die_list_count):
    high_number = 100000
    counter = len(die_list) * [0]
    for __ in range(high_number):
        dieres = [
            sum(roll(die) for __ in range(repeat))
            for (die, repeat) in zip(die_list, die_list_count)
        ]

        maximum = max(dieres)
        if dieres.count(maximum) == 1:
            winner = dieres.index(maximum)
            counter[winner] += 1

    return [
        str(count / high_number) + '% for die' + str(i)
        for (i, count) in enumerate(counter, start=1)
    ]

main

• while Doing is redundant: Doing is just set to False at the end of loop :/

• if len(dietoadd) >= 1 should be replaced with if dietoadd. Actually, it is better to move this check inside deeper ("read die" function), so die_list_2 could be read much simpler:

  die_list_2 = []
  while input('to stop enter S') != 'S':
      die_list_2.append(deeper('Die' + str(len(die_list_2))))
  

• while quantity loop may be removed because you can construct Qlist in a single line:

  Qlist = [suckmypp(counterq) for counterq in range(len(die_list_2))]
  

• inline variables that are used only once, e. g. chance and chance2 could be removed if you call chance_overlap inside print().

So, after all refactoring main part should look like:

die_list_1 = [Die1, Die2]
diecount = [9, 6]
print(chance_overlap(die_list_1, diecount))

die_list_2 = []
while input('to stop enter "S"') != 'S':
    die_list_2.append(deeper('Die' + str(len(die_list_2))))

q_list = [suckmypp(counterq) for counterq in range(len(die_list_2))]

print(die_list_2)
print(q_list)
print(chance_overlap(die_list_2, q_list))

Answer 2

I haven't been brave enough to read through all the code. If your variables and function are to have irrelevant names, then I'd highly recommend commenting your code so that people will be more willing to read it.

I have however noticed one thing : the function roll can be replaced by the use of randrange(die) + 1, and therefore the variables Die1 and Die2 need only be an integer, representing the maximum value of the die.

Also, if you create lists -like Die1 and Die2 here-, think about using range() and list comprehensions : Die1 = [x+1 for x in range(4)]. If you want to cast a d100, it'll be easier than hardcoding each number yourself (would still go with random.randrange(100) + 1, though)

Here's the documentation for the randrange function in the random module : https://docs.python.org/3/library/random.html#random.randrange