Skip to content

Conversation

@drakeblood4
Copy link

I think this sorting method is better when the person you're trading to has more wants than they have points. There's probably a better way to write it than a mess of for loops, but I don't know it off the top of my head.

@drakeblood4
Copy link
Author

Here's a simple use case where this would provide a higher value bundle than the currently default sorting program:

User: Frank
Total points: 1000
Wants: 700 points, 450 points, 450 points, 350 points, 200 points.

Default bundle order: 700, 450, 450, 350, 200

Optimized bundle order: 450, 350, 200, 750, 450

sorted_cards = sorted(cards, key=lambda k: k["value"], reverse=True)
else:
largest = 0
sorted_cards = []
Copy link
Owner

@tomreece tomreece Jul 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorted_cards has already been initialized to [] above on line 336. Do we need this line?

@tomreece
Copy link
Owner

I like the idea of this change a lot.

I added some basic comments that I believe help improve the clarity of the code. If you don't feel like or have time to act on them, no worries. I can pull down your code and make the changes. I'd also like to see if we could implement the same plan with less loops as you mention.

Great idea! Let me know if you want to refactor it a bit or if you want me to run with it from here. You've given me a great start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants