HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonModerate

Printing all possible sums of two n-sided dice rolls

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
allrollspossibleprintingtwodicesumssided

Problem

For my CS class, we were tasked with writing a program that calculates all the possible results from rolling two n-sided dice. For example, the input of 4 would print [2, 3, 3, 4, 4, 4, 5, 5, 5, 5, 6, 6, 6, 7, 7, 8]. Or, if the input was 3, it would print [2, 3, 3, 4, 4, 4, 5, 5, 6].

I was given enough test cases that I noticed the pattern that the number range was [2,n] and the test cases had a pattern where the pattern was always could be represented by printing the number in [2,2n+1] a number in the set [1,n] U (n,1] a corresponding number of times. For example, for 4, the set would be [1,2,3,4,3,2,1] and the output would be [(one 2), (two 3s), (three 4), (four 5s), (three 6s), (two 7s), and (one 8). This explanation may be horrible, but see the code and you will understand.

The code:

def allRolls(sides):
    nums = list(range(1,sides+1)) + reverseIt(list(range(1,sides)))
    index = sides-1
    #the return array
    retVal = []
    #The index to look for in the list of nuts
    cIndex = 0
    for i in range(2,sides*2+1):
        for b in range(nums[cIndex]):
            retVal.append(i)
        cIndex+=1
    retVal.sort()
    return retVal

#Reverses the list
def reverseIt(liste):
    newb =[]
    index = len(liste)-1
    while index>=0:
        newb.append(liste[index])
        index= index-1
    return newb


The issue: everyone in my class got it in many fewer lines of code. What did I do wrong?

Code test cases:

Input: 2

Output:[2, 3, 3, 4]

Input: 3

Output:[2, 3, 3, 4, 4, 4, 5, 5, 6]

Input: 4

Output: [2, 3, 3, 4, 4, 4, 5, 5, 5, 5, 6, 6, 6, 7, 7, 8]

Solution

As @BenC mostly put a code dump, I'll explain ways that your code could have improved.

reverseIt

Programming this is just wrong in Python.
This is as there are two ways to do this.

>>> list(reversed([1, 2, 3]))
[3, 2, 1]
>>> [1, 2, 3][::-1]
[3, 2, 1]


As an alternate way to this you could just calculate the range backwards off the bat.

>>> list(range(10, 0, -1))
[10, 9, 8, 7, 6, 5, 4, 3, 2, 1]


In short you could halve your code just by removing this.

If you wanted to keep it, then you would want to use the reversed range.
And do pretty much what you are now. To get:

def reverseIt(indexable):
    lst = []
    for index in range(len(indexable) - 1, -1, -1):
        lst.append(indexable[index])
    return lst


You could then use a list comprehension, if amount of lines dictates how good your code is.

def reverseIt(indexable):
    return [indexable[index] for index in range(len(indexable) - 1, -1, -1))]


allRolls

  • nums is pointless if you use a different algorithm. (Which is also more intuitive.)



  • index is never used.



  • retVal can be changed to a comprehension.



  • Manually counting cIndex is a frowned upon in Python.



In short, use a different algorithm.
The pseudo-code for this could be:

fn allRolls(sides):
    list = List()
    for dice1-face in dice-faces:
        for dice2-face in dice-faces:
            add dice1-face + dice2-face to list
    return sorted list


Or in Python:

fn allRoles(sides):
    ret = []
    for a in range(1, sides + 1):
        for b in range(1, sides + 1):
            ret.append(a + b)
    ret.sort()
    return ret


If you take it the next step then you would get @BenC's answer.

Misc

In short, you just used the wrong algorithm.

And in Python you should:

  • Use all_rolls instead of allRoles to follow function and variable naming conventions.



  • Have a space on both sides of operators, index >= 0 or newb = [] to increase readability.

Code Snippets

>>> list(reversed([1, 2, 3]))
[3, 2, 1]
>>> [1, 2, 3][::-1]
[3, 2, 1]
>>> list(range(10, 0, -1))
[10, 9, 8, 7, 6, 5, 4, 3, 2, 1]
def reverseIt(indexable):
    lst = []
    for index in range(len(indexable) - 1, -1, -1):
        lst.append(indexable[index])
    return lst
def reverseIt(indexable):
    return [indexable[index] for index in range(len(indexable) - 1, -1, -1))]
fn allRolls(sides):
    list = List()
    for dice1-face in dice-faces:
        for dice2-face in dice-faces:
            add dice1-face + dice2-face to list
    return sorted list

Context

StackExchange Code Review Q#114584, answer score: 16

Revisions (0)

No revisions yet.