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

Memory leaks in lottery simulator

Submitted by: @import:stackexchange-codereview··
0
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

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 = 0


In 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 = numbers


You 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 = 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 ) ):

Context

StackExchange Code Review Q#4098, answer score: 12

Revisions (0)

No revisions yet.