#1
  1. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2013
    Posts
    2
    Rep Power
    0

    Working program but review required to make it better


    Hi All,

    I just started learning Python using online resources and am just a couple of days old in it. I have quite a good experience in Java though (10+ years) and Python caught my fancy during the Christmas holidays as had nothing better to do

    I wrote a quick and dirty code to compute factorial of a number using Python and I need an expert's opinion about it with respect to the structure and semantics - is there something that can be more optimized in it, something that was done incorrectly etc. Am interested to learn the language right the first time, and will immensely appreciate any feedback.

    The idea of code is simple - being a recursive algorithm, I've tried to cache all previously computed values of factorial in a file which is cached when an object of FactCache is created. Factorial then uses the cache to reduce recursion.

    Code follows:

    Code:
    import os
    import pickle
    """
    Caches and pickles factorial of a number.
    Upon instantiation, sarches for the file 'fact.cache'
    in the current directory or in the path passed during
    construction. If not found then creates the file there.
    If found then unpickles it and creates the dictionary
    with pre-calculated factorial values.
    """
    class FactCache:
        c = {}
        def __init__(self,path="./fact.cache"):
            self.path=path
            try:
                if(os.path.isfile(self.path)):
                    print("Cache file found. Reading it now...")
                    with open(self.path,'rb') as readCache:
                        self.c = pickle.load(readCache)
                else:
                    print("File not found. Creating it now...")
                    with open(self.path,'wb') as writeCache:
                        pickle.dump(self.c,writeCache)
            except IOError as err:
                print("Oops! Factorial cache not loaded!",str(err))
                pass
            finally:
                print(self.c)
    
        """
        Stores the value (f) of the factorial for a number (n) in the file.
        """
        def add(self,n,f):
            self.c[n]=f
            with open(self.path,'wb') as writeCache:
                pickle.dump(self.c,writeCache)    
    
        """
        Returns the value of the factorial for a number (n) from the dictionary.
        """
        def get(self,n):
            if n in self.c:
                return self.c[n]
            else:
                return -1
    
    """
    Computes factorial of a given number and
    reuses all previous results to reduce recursion.
    """
    def factorial(n,c):
        f = c.get(n)
        if(f == -1):
            if(n==0):
                return 1
            else:
                v = n*factorial(n-1,c)
                c.add(n,v)
                return v
        else:
            print("Value served from cache for ",n)
            return f
    Output follows:
    >>> factorial(26,c)
    Value served from cache for 25
    403291461126605635584000000
  2. #2
  3. Contributing User
    Devshed Demi-God (4500 - 4999 posts)

    Join Date
    Aug 2011
    Posts
    4,997
    Rep Power
    481
    Wonderful.

    Next write a decorator to hide the cache argument.
    [code]Code tags[/code] are essential for python code and Makefiles!
  4. #3
  5. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jul 2007
    Location
    Joensuu, Finland
    Posts
    439
    Rep Power
    67
    Originally Posted by MickeyCoder
    Am interested to learn the language right the first time, and will immensely appreciate any feedback.
    Docstrings should go below the thing they document, not (as in Ruby) above. Also, PEP 257 recommends that the first line is followed by a blank line. I.e.:

    Code:
    def spam(eggs):
        '''Add spam to my eggs.
    
        A more detailed explanation follows here.
        '''
    My armada: openSUSE 13.2 (home desktop, work desktop), openSUSE 13.1 (home laptop), Debian GNU/Linux 7.7.0 (mini laptop), Ubuntu 14.04 LTS (server), Android 4.2.1 (tablet), Windows 7 Ultimate (testbed)
  6. #4
  7. Contributing User
    Devshed Novice (500 - 999 posts)

    Join Date
    Feb 2005
    Posts
    620
    Rep Power
    65
    You can also access a properly located documentation string this way:
    Code:
    ''' DocumentationString1.py
    documentation strings must be added right below the function definition,
    triple quotes are recommended as a standard
    can be accessed via function_name.__doc__
    '''
    
    def get_distance(x1, y1, x2, y2):
        """
        get_distance(x1, y1, x2, y2)
        returns distance between two points using the pythagorean theorem
        the function parameters are the coordinates of the two points
        """
        dx = x2 - x1
        dy = y2 - y1
        return (dx**2 + dy**2)**0.5
    
    #
    # since the indentation rules relax between triple quotes
    # you can also use this to avoid adding spaces in front of 
    # the doc string
    #
    
    def get_distance2(x1, y1, x2, y2):
        """
    get_distance(x1, y1, x2, y2)
    returns distance between two points using the pythagorean theorem
    the function parameters are the coordinates of the two points
        """
        dx = x2 - x1
        dy = y2 - y1
        return (dx**2 + dy**2)**0.5
    
    print( "The function's documentation string:" )
    # shows text between the triple quotes
    print(get_distance.__doc__)
    print("\nWithout leading spaces:")
    print(get_distance2.__doc__)
    
    ''' my result >>>
    The function's documentation string:
    
        get_distance(x1, y1, x2, y2)
        returns distance between two points using the pythagorean theorem
        the function parameters are the coordinates of the two points
        
    
    Without leading spaces:
    
    get_distance(x1, y1, x2, y2)
    returns distance between two points using the pythagorean theorem
    the function parameters are the coordinates of the two points
    '''
    Real Programmers always confuse Christmas and Halloween because Oct31 == Dec25
  8. #5
  9. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2013
    Posts
    2
    Rep Power
    0

    Thanks!


    Thank you all for your feedback. @b49P23TIvg: The decorators are very interesting and thanks for pointing me in that direction!
  10. #6
  11. Contributing User
    Devshed Novice (500 - 999 posts)

    Join Date
    Feb 2005
    Posts
    620
    Rep Power
    65
    A decorator would be a little overkill since it would only be useful for that particular function.
    Real Programmers always confuse Christmas and Halloween because Oct31 == Dec25

IMN logo majestic logo threadwatch logo seochat tools logo