3
\$\begingroup\$

It's a dice game between 2 people using tricky dice. For example,three dice [[1, 1, 4, 6, 7, 8], [2, 2, 2, 6, 7, 7], [3, 3, 3, 5, 5, 8]].

The code will decide, in order to win, who chooses the dice first and which die to play. Say if you choose a die first, return the index of the die. Say if you decide to be the second one to choose a die, then specify for each die that your opponent may take, and the die that you would take in return.

The code works but please tell me what you think or how to improve it!

from itertools import product

def count_wins(dice1, dice2):
    assert len(dice1) == 6 and len(dice2) == 6
    dice1_wins, dice2_wins = 0, 0
    for p in product(dice1,dice2):
      if p[0]>p[1]:
        dice1_wins += 1
      elif p[0]<p[1]:
        dice2_wins += 1
    return (dice1_wins, dice2_wins)

def find_the_best_dice(dices):
    assert all(len(dice) == 6 for dice in dices)
    adj=[[]for _ in range(len(dices))]
    for i in range(len(dices)-1):
      for j in range(i+1,len(dices)):
        a,b=count_wins(dices[i],dices[j])
        print('ij',i,j,'ab',a,b)
        if a<b:
          adj[i].append(j)
        elif a>b:
          adj[j].append(i)
        else:
          adj[j].append(i)
          adj[i].append(j)
    ans=-1 
    for i in range(len(adj)):
      if len(adj[i])==0:
        ans=i
    return ans,adj

def compute_strategy(dices):
    assert all(len(dice) == 6 for dice in dices)
    ans,adj=find_the_best_dice(dices)
    print(ans,adj)
    strategy = dict()
    strategy["choose_first"] = True
    strategy["first_dice"] = 0
    if ans!=-1:
      strategy["first_dice"] = ans
    else:
      strategy.pop("first_dice")
      strategy["choose_first"] = False
      for i in range(len(dices)):
        for k in adj[i]:
          a,b=count_wins(dices[i],dices[k])
          if a!=b:
            strategy[i]=k
            break
    return strategy
\$\endgroup\$
2
  • 1
    \$\begingroup\$ The plural of "die" is "dice", and "dices" is not a word (yes, English is ridiculous) \$\endgroup\$
    – Reinderien
    Aug 28, 2021 at 8:41
  • \$\begingroup\$ Thanks I didn't know!! \$\endgroup\$
    – Chris
    Aug 28, 2021 at 11:23

1 Answer 1

5
\$\begingroup\$

Variable names

Some names are not clear enough. What p means, a pair? You don't need a pair:

for p in product(dice1,dice2): => for side1, side2 in product(dice1,dice2):

a,b=count_wins(dices[i],dices[j]) => wins1, wins2 = count_wins(dices[i],dices[j])

What 'adj' means? "Adjacent"? Maybe it should be "wins" or something like this. Make the code readable.

Type hints

Use them.

Indents are important not only in the beginning

Put spaces at least right to commas and on the both sides of operators - or at least do it consistently. Sometimes you do, sometimes you don't. That irritating.

Use Python features more intensively

assert len(dice1) == 6 == len(dice2) #chain compare
....
adj=[[] for _ in dices] #you don't need range(len()) if you don't need a number
....
for i, wins in enumerate(adj): #"enumerate" generates index-value pairs
  if not wins: #"len(...) == 0" is just "not ..."
    ans = i

You can make count_wins function much shorter and readable, counting only wins, not losses. This will make you call it twice, but \$O(2n)==2O(n)==O(n)\$, and \$n\$ is 6, so you lose nothing.

def count_wins(dice1, dice2):
    assert len(dice1) == 6 and len(dice2) == 6
    return sum(1 for side1, side2 in product(dice1,dice2) if side1>side2)

Or you can do this twice inside count_wins - both ways.

Unnecessary and insufficient asserts

This code looks like there's only one entry point, at compute_strategy. So other functions probably shouldn't check for list sizes. On the other hand, you don't check many other things like type of input - is it a list of lists of integers? Can there be negative values? There's nothing wrong in extra checking, but if you are validating data - validate it, not just check one assertion several times.

You can make it a class to make sure all inputs would go through an __init__ method.

Remove debugging output before showing your code to others

I don't think prints belong here, though they could be very useful while debugging.

Optimization ideas

I don't think comparing 6-element lists needs an optimization, but you still can sort the sides and then bisect search the number of elements less than the other side. This will give you \$O(n*log(n))\$ time instead of yours \$O(n^2)\$ product. Once again - just FYI.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.