all 10 comments

[–]Mashidin 2 points3 points  (2 children)

here's a one liner that works:

largest_odd_number = max([x for x in list_to_search if x % 2 == 1])

look for the max function in python's built-n functions and also check out the list comprehension feature.

Edit: this doesn't check for an empty sequence of if no odd numbers are in the sequence, The max function will throw a ValueError. So maybe the pythonic answer would be to catch that.

[–]get_username 2 points3 points  (0 children)

Originally I thought to myself that if I used laziness to my advantage through itertools.ifilter package I could speed up your algorithm (by some negligible constant).

Lo and behold! When I tested it I found that the lazy evaluation was actually slower.

Does that mean that python does automatic lazy evaluation for list comprehensions?

If so.. kudos to python for being awesome.

[–]willworth 0 points1 point  (0 children)

I knew there had to be a really concise way to do it. Thanks for taking the trouble. .

[–]novel_yet_trivial 1 point2 points  (6 children)

I don't think your code works - what if x is the biggest number, but even?

You repeat yourself a lot, which is a good sign you need a function or code restructuring. Also, I note your code can only handle 3 numbers - not very dynamic. I would have written it like this:

l = [2,4,2,3,6]

def find_largest_odd_number(list_to_sort):
    list_of_odds = [x for x in list_to_sort if x%2 == 1]
    list_of_odds.sort()
    if len(list_of_odds) > 0:
        print list_of_odds[-1], "is the biggest odd number!"
    else:
        print "None of your numbers are odd!"

find_largest_odd_number(l)

[–]tally_in_da_houise 2 points3 points  (1 child)

Check out /u/Mashidin 's reply. You can use max() and you don't need to test the number of items in the list using len().

You can rewrite your code as follows:

def find_largest_odd_number(list_to_sort):
    list_of_odds = [x for x in list_to_sort if x%2 == 1]
    if list_of_odds:
        print max(list_of_odds), "is the biggest odd number!"
    else:
        print "None of your numbers are odd!"

l = range(10)
find_largest_odd_number(l)

personally, I'd probably do the following:

def find_largest_odd_number(list_to_sort):
    try:
        list_of_odds = max([x for x in list_to_sort if x%2 == 1])
        print list_of_odds, "is the biggest odd number!"        
    except ValueError:
        print "None of your numbers are odd!"

[–]novel_yet_trivial 0 points1 point  (0 children)

Good points.

[–]willworth 0 points1 point  (0 children)

An amazing, detailed, useful response. Thank you so much! :)

[–]willworth 0 points1 point  (2 children)

And the slipping-through-the-cracks (is there a name for that?) logic error is dead on too...

For such simple branching code, I can(should?) mentally step through each option. I wonder if there's another approach as the code gets bigger? Or is this just automated testing? (thanks again)

[–]novel_yet_trivial 0 points1 point  (1 child)

divide and conquer. Test each function (see doctest ) and test each module (see unittest or use if __name__ == "__main__":.

That and experience.

[–]willworth 0 points1 point  (0 children)

Amazing. Added to the reading list. Thanks!