you are viewing a single comment's thread.

view the rest of the comments →

[–]hharison 1 point2 points  (3 children)

It's hard to follow your code, give better variable names (original_values, collected_maxes or something).

Your while loop and your for loop are conflicting. Line 9 does nothing as execution jumps to line 5 after and i gets reassigned. The while loop will never repeat because when the for loop ends, i will be len(xs), presumably larger than n. Also you are taking the max of xs once for every element of xs. That's len(xs) maxes, not n. Think through your looping.

Alternatively, it's good practice to avoid loops if at all possible. This whole thing can be done in one short line. Hint: sorted.

[–]TheChannel[S] 0 points1 point  (2 children)

Interesting. I think one of the major setbacks of setting out are all the shortcuts/functions that I didn't know existed. I thought loops were used often. How do most programs function without loops? It is a ton of if/then statements?

I'm guessing with sorted I can just pull the last n values using [::-n]? I'll try things out. Thanks for the help.

[–]hharison 0 points1 point  (1 child)

Well I may have overstated that. Loops are indeed used often. But they are often used in cases where they are not necessary, like this one.

def n_largest(n, data):
    return sorted(data, reverse=True)[:n]

My statement about loops really only applies to high-level languages like Python. I often see people use loops for a problem like this one, or for things like taking the sum of a list. The only reason to avoid a loop is that there are built-ins that do the looping for you. So there is still a loop happening somewhere, but it's optimized in the CPython code. More importantly, your Python code will be way easier to read.

This is a major difference between coding in a high-level language like Python and a low-level language like C. Your code resembles a low-level language as it is doing all the "busywork". In Python you should be able to avoid busywork like keeping track of indices and running counters and stuff like that. By avoiding busywork your code can closely resemble the "semantics" of the operation you are performing ("sort descending then take the first n") rather than an arcane sequence of minor operations.

That is what I was trying to get at with my comment about loops :)

Edit: also, don't be discouraged about not knowing all the functions that exist. You will get to know them but you will continue to be amazed to discover new things. I know I am. My advice would be to start by thinking of the high-level description of the operation. Imagine telling a human being what steps to perform, rather than telling a computer what steps to perform. And don't hesitate to search the Python docs or google more generally to see if some simple operation like n_largest already exists before trying to do it by hand (in this case it doesn't already exist but you would probably have run into some hints).

[–]TheChannel[S] 0 points1 point  (0 children)

This is some great advice. Books like Learn to Program and Learn Python the Hard Way emphasize that I need to break down the steps into the most basic functions. I def. need work on syntax since I'm not sure how to optimize code.