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

Matrix of heights test

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

Problem

I was writing a program for my math class because my Professor asked me to. The problem is as follows: 300 people are arranged in 30 rows each of 10 people. The tallest of each row is chosen, and the shortest of each column. The problem is to prove whether the shortest of the taller people is taller, or the tallest of the shorter people. My program is as follows, but 90% of my coding standards are from PPCG, so my code is likely awful.

import random

multiple = int(input("Would you like multiple runs or one (enter 1 or 0)?\n").rstrip())

def weighted_random():
# between 3 ft and 6 ft
temp = random.random()
t = 0
if temp > 0.95:
t = 6
elif temp = tall_man:

zeroes += 1

print("Out of %d evaluations, we got %d results as shortest of tall taller than tallest of short, and %d times they were equal." % (times, times - zeroes, zeroes))

Solution

An old question, bet let's leave no question unanswered... I will go through your code (mostly) in order in which it is presented.

Asking for multiple is, in my opinion, in the wrong position, as it is separated by functions definitions from the rest of the main code. I'd move it after the functions definitions.

Closely related to that, asking the user to answer with C-like logic of 1 for "yes" or 0 for "no" is not pleasant, especially since Python doesn't share C's woes with strings. Btw, int(...) has no problem with extra spaces, so (r)stripping is not needed.

And, since we're becoming user-friendly here, let's allow users to make typos without crashes and other unexpected behaviour by asking them again if they give a nonsense answer. All together:

while True:
    answer = input("Would you like answer runs (y/n)? ").strip().lower()
    if answer in {"y", "n", "yes", "no"}:
        break
multiple = (answer in {"y", "yes"})


Creating weighted randoms can be done in a more concise and configurable way. I have chosen this:

weights = [0, 0, 0, 0.1, 0.5, 0.95, 1]
def weighted_random():
    # between 3 ft and 6 ft
    major = random.random()
    major = next(idx for idx,limit in enumerate(weights) if limit > major)
    return major + random.random()


The weights list (could've been a tuple as well) contains limits. The major value (the integer part) of the height is the index of the first value bigger than the random that was picked. So, for example, 0.05 is between 0 and 0.1, meaning that we pick major = 3 because 3 is the index of 0.1.

Apart from keeping the code short, this makes it trivial to change the weights, including adding more weight categories, as long as we keep to the premise that these define the integer part. Otherwise, we could've used a dictionary or some similar approach.

We create heights twice in the program, so let's not copy/paste this non-trivial and important code. A function is better:

def make_heights(rows=30, cols=30):
    return [[weighted_random() for j in range(cols)] for i in range(rows)]


Printing is also best done using generators and joining the elements with the space character:

def output_heights(data):
    print("Men:")
    for row in data:
        print(" ".join("%.3f" % el for el in row))


Your _zip function confused me at first. There is a zip() in Python, but it does something far more reasonable for a function with such a name. Consider renaming this to "transpose".

Either way, I think it's best to handle "the shortest of the tall ones" and "the tallest of the short ones" the same way, with a single function for each of them:

def shortest_of_talls(data):
    return min(max(row) for row in data)

def tallest_of_shorts(data):
    return max(min(row[j] for row in data) for j,_ in enumerate(data[0]))


Here, I use enumerate to get (index, value) tuples, and I use a throwaway variable _ for values because I don't need them. There are a few situations when _ is not a good choice, but that's beyond the scope of this review.

Btw, notice that I use data. It's more descriptive than array (albeit, heights or people might have been a better choice).

Your main if looks like this: if not multiple: ... else: .... Does it not make more sense to do if multiple: ... else: ...? I changed it thus, so let's first review the "multiple" code.

I prefer evalf = list() over evalf = [], as I find it more readable. You're more likely to notice a bug if you put set() instead of list() than if you put {} instead of [].

We decided above to be more user friendly, so let's keep that up when asking about the number of runs. You wanted to ask the user to give you an integer, and default to 10000 if they give 0. Here is the code that takes an empty(ish) string (we allow spaces) which is treated as the default, or a positive integer. Any other response will repeat the question:

while True:
        times = input("Number of evaluations (leave empty for 10000)? ")
        if times.strip() == "":
            times = 10000
            break
        try:
            times = int(times)
            if times > 0:
                break
        except ValueError:
            pass


The next part of your code is biased toward what you know (expect?) is the right answer. If I got it right, this is a research question, so let's research the results properly, by counting how many "tallest of shorts", "shortest of talls", and "equals" are there:

cnt_tos = cnt_sot = cnt_eq = 0


Since your default is to run 10000 tests, and that takes time (roughly a minute on my laptop), I decided to add progress information here. It's a neat little trick: print a message, but end with "\r" instead of "\n" to place the cursor at the beginning of the same line, instead of the next one:

```
cnt_tos = cnt_sot = cnt_eq = 0
done = -1
for run in range(times):
new_done = 100*run // times
if

Code Snippets

while True:
    answer = input("Would you like answer runs (y/n)? ").strip().lower()
    if answer in {"y", "n", "yes", "no"}:
        break
multiple = (answer in {"y", "yes"})
weights = [0, 0, 0, 0.1, 0.5, 0.95, 1]
def weighted_random():
    # between 3 ft and 6 ft
    major = random.random()
    major = next(idx for idx,limit in enumerate(weights) if limit > major)
    return major + random.random()
def make_heights(rows=30, cols=30):
    return [[weighted_random() for j in range(cols)] for i in range(rows)]
def output_heights(data):
    print("Men:")
    for row in data:
        print(" ".join("%.3f" % el for el in row))
def shortest_of_talls(data):
    return min(max(row) for row in data)

def tallest_of_shorts(data):
    return max(min(row[j] for row in data) for j,_ in enumerate(data[0]))

Context

StackExchange Code Review Q#129144, answer score: 4

Revisions (0)

No revisions yet.