patternpythonModerate
Memory leaks in lottery simulator
Viewed 0 times
simulatorleakslotterymemory
Problem
I'm primarily a C++/Java programmer, but I've recently started using Python at work and decided to write a Lottery Simulator at home. I wrote it to test out different combinations of lottery numbers against a set of winning numbers to calculate how much I would have won (I also run the lotto pool at work). The only major problem I've seen with the code is that it has a substantial memory leak that I just can't seem to track down. I've already plugged one large leak.
I'd really appreciate any help you can provide in tracking down the leak(s). The code is pretty large (655 lines), but a lot of it is comments & blank lines for readability. Also feel free to use the code yourself if you want, I'm not copyrighting it.
```
#!/usr/bin/python
import random, ConfigParser, ast, copy
from optparse import OptionParser
# Global variables.
g_config = {} # Stores GameType config options.
g_prizes = {} # Stores prizes from config file.
g_prize_amounts = {} # Stores prize amounts from config file.
g_ticket_quick_picks = [] # Store an array of QuickPicks to use for tickets.
g_total_money_won = 0
g_total_tickets_won = 0
g_prizes_won = {} # Format: {'3':3, '3+1':2, '4':2 ...}
g_quick_pick_file = ''
g_how_many_tickets_played = 0
g_how_many_tickets_won = 0
class TicketNumbers( object ):
'''Represents the numbers of a ticket (or the 7 winning numbers).
@var numbers: A sorted array of 7 unique numbers from 1-49.'''
def __init__( self, numbers ):
'''Constructor.
@param numbers: The numbers for this ticket.'''
self.numbers = numbers
self._set_numbers( numbers )
def name( self ):
'''Return the name of this class type.'''
return "TicketNumbers"
def __str__( self ):
'''Returns a string of the number list.'''
str_nums = []
for i in range( len( self.numbers ) ):
str_nums.append( str( self.numbers[i] ) )
return ', '.join( str_nums )
def _set_numbers( self, numbers ):
'''Sets the numbers member
I'd really appreciate any help you can provide in tracking down the leak(s). The code is pretty large (655 lines), but a lot of it is comments & blank lines for readability. Also feel free to use the code yourself if you want, I'm not copyrighting it.
```
#!/usr/bin/python
import random, ConfigParser, ast, copy
from optparse import OptionParser
# Global variables.
g_config = {} # Stores GameType config options.
g_prizes = {} # Stores prizes from config file.
g_prize_amounts = {} # Stores prize amounts from config file.
g_ticket_quick_picks = [] # Store an array of QuickPicks to use for tickets.
g_total_money_won = 0
g_total_tickets_won = 0
g_prizes_won = {} # Format: {'3':3, '3+1':2, '4':2 ...}
g_quick_pick_file = ''
g_how_many_tickets_played = 0
g_how_many_tickets_won = 0
class TicketNumbers( object ):
'''Represents the numbers of a ticket (or the 7 winning numbers).
@var numbers: A sorted array of 7 unique numbers from 1-49.'''
def __init__( self, numbers ):
'''Constructor.
@param numbers: The numbers for this ticket.'''
self.numbers = numbers
self._set_numbers( numbers )
def name( self ):
'''Return the name of this class type.'''
return "TicketNumbers"
def __str__( self ):
'''Returns a string of the number list.'''
str_nums = []
for i in range( len( self.numbers ) ):
str_nums.append( str( self.numbers[i] ) )
return ', '.join( str_nums )
def _set_numbers( self, numbers ):
'''Sets the numbers member
Solution
#!/usr/bin/python
import random, ConfigParser, ast, copy
from optparse import OptionParser
# Global variables.
g_config = {} # Stores GameType config options.
g_prizes = {} # Stores prizes from config file.
g_prize_amounts = {} # Stores prize amounts from config file.
g_ticket_quick_picks = [] # Store an array of QuickPicks to use for tickets.
g_total_money_won = 0
g_total_tickets_won = 0
g_prizes_won = {} # Format: {'3':3, '3+1':2, '4':2 ...}
g_quick_pick_file = ''
g_how_many_tickets_played = 0
g_how_many_tickets_won = 0In Python, it is recommended not have global variables. I usually only put constants at the global level, and according to python style guide they should be written WITH_ALL_CAPS. I'd move all of this data into some sort of object.
class TicketNumbers( object ):
'''Represents the numbers of a ticket (or the 7 winning numbers).
@var numbers: A sorted array of 7 unique numbers from 1-49.'''Considering the fact that you later sort the number, why do specify that it should be sorted on input?
def __init__( self, numbers ):
'''Constructor.
@param numbers: The numbers for this ticket.'''
self.numbers = numbersYou assign to numbers here, but _set_numbers also assigns to it. This one is pointless.
self._set_numbers( numbers )
def name( self ):
'''Return the name of this class type.'''
return "TicketNumbers"What's the point of this? If you want the name of the class use
obj.__class__.__name__def __str__( self ):
'''Returns a string of the number list.'''
str_nums = []
for i in range( len( self.numbers ) ):There is almost never a need to use the range( len( container pattern. Just iterate over the numbers directly.
str_nums.append( str( self.numbers[i] ) )There are a number of ways of writing this loop more simply:
str_nums = [str(number) for number in self.numbers]
str_nums = map(str, self.numbers)
return ', '.join( str_nums )Actually, the entire function can be written as:
return ', '.join(str(number) for number in self.numbers)
def _set_numbers( self, numbers ):
'''Sets the numbers member variable and validates the numbers.
@param numbers: The numbers to assign.'''
global g_config
self.numbers = sorted( numbers )
if len( self.numbers ) != int(g_config['how_many_numbers']):
raise Exception( "You must have %d numbers, but found %d numbers: %s" % (g_config['how_many_numbers'], len( self.numbers ), str(self.numbers)) )If you are validating user input, I suggest create a custom exception class. You should then catch this custom class and print just the error message instead of having python's default. Your error message should also give a better indication of where the numbers are from.
tmp = []tmp is a terrible variable name, because all local variables are temporary.
for num in self.numbers:
# Check if numbers are in range.
if ( num int(g_config['max_number']) ):I recommend pulling configuration data into the correct types inside your configuration logic not in the rest of your code.
raise Exception( "The numbers must be between %d & %d!" % (int(g_config['min_number']), int(g_config['max_number'])) )
# Check for duplicate numbers.
if num in tmp:
raise Exception( "You cannot have duplicate numbers! %d in %s" % (num, str( self.numbers )) )
tmp.append( num )I suggest splitting these check out into three seperate bits:
if min(self.numbers) config.max_number:
raise UserError("Too high!")
if len(set(self.numbers)) != len(self.numbers):
raise UserError("duplicates!")
class PlayedTicket( TicketNumbers ):
'''Represents a ticket that was played.
@var amount_won: How much $ this ticket won.'''A ticket isn't a special kind of TicketNumbers. The names suggest that a ticket should contain numbers.
def __init__( self, numbers ):
'''Constructor.
@param numbers: The numbers for this ticket.'''
super( PlayedTicket, self ).__init__( numbers )
self.amount_won = {} # Map of Game Index to $ won each play.
self.tickets_won = {} # Map of Game Index to Free Tickets won each play.
def name( self ):
'''Return the name of this class type.'''
return "PlayedTicket"
def compare_with( self, winning_numbers, game_index=None ):
'''Compares this ticket with the specified winning numbers and sets the amount_won & tickets_won members accordingly.
@param winning_numbers: The winning numbers to compare against.
@param game_index: (optional) The game index for this game. Defaults to 1 + the number of games recorded.
@return: A tuple of ($ won, # free tickets won).'''The name compare_with doesn't suggest
Code Snippets
#!/usr/bin/python
import random, ConfigParser, ast, copy
from optparse import OptionParser
# Global variables.
g_config = {} # Stores GameType config options.
g_prizes = {} # Stores prizes from config file.
g_prize_amounts = {} # Stores prize amounts from config file.
g_ticket_quick_picks = [] # Store an array of QuickPicks to use for tickets.
g_total_money_won = 0
g_total_tickets_won = 0
g_prizes_won = {} # Format: {'3':3, '3+1':2, '4':2 ...}
g_quick_pick_file = ''
g_how_many_tickets_played = 0
g_how_many_tickets_won = 0class TicketNumbers( object ):
'''Represents the numbers of a ticket (or the 7 winning numbers).
@var numbers: A sorted array of 7 unique numbers from 1-49.'''def __init__( self, numbers ):
'''Constructor.
@param numbers: The numbers for this ticket.'''
self.numbers = numbersself._set_numbers( numbers )
def name( self ):
'''Return the name of this class type.'''
return "TicketNumbers"def __str__( self ):
'''Returns a string of the number list.'''
str_nums = []
for i in range( len( self.numbers ) ):Context
StackExchange Code Review Q#4098, answer score: 12
Revisions (0)
No revisions yet.