Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This is my try at the proxy pattern.

What do you Pythoneers think of my attempt?

class Image:
    def __init__( self, filename ):
        self._filename = filename
    def load_image_from_disk( self ):
        print("loading " + self._filename )
    def display_image( self ):
        print("display " + self._filename)

class Proxy:
    def __init__( self, subject ):
        self._subject = subject
        self._proxystate = None

class ProxyImage( Proxy ):
    def display_image( self ):
        if self._proxystate == None:
            self._subject.load_image_from_disk()
            self._proxystate = 1
        print("display " + self._subject._filename )

proxy_image1 = ProxyImage ( Image("HiRes_10Mb_Photo1") )
proxy_image2 = ProxyImage ( Image("HiRes_10Mb_Photo2") )


proxy_image1.display_image() # loading necessary
proxy_image1.display_image() # loading unnecessary
proxy_image2.display_image() # loading necessary
proxy_image2.display_image() # loading unnecessary
proxy_image1.display_image() # loading unnecessary

Output:

loading HiRes_10Mb_Photo1
display HiRes_10Mb_Photo1
display HiRes_10Mb_Photo1
loading HiRes_10Mb_Photo2
display HiRes_10Mb_Photo2
display HiRes_10Mb_Photo2
display HiRes_10Mb_Photo1
share|improve this question
add comment

1 Answer

up vote 5 down vote accepted

It seems like overkill to implement a class to represent the proxy pattern. Design patterns are patterns, not classes. What do you gain from your implementation compared to a simple approach like the one shown below?

class Image(object):
    def __init__(self, filename):
        self._filename = filename
        self._loaded = False

    def load(self):
        print("loading {}".format(self._filename))
        self._loaded = True

    def display(self):
        if not self._loaded:
            self.load()
        print("displaying {}".format(self._filename))

Some other notes on your code:

  1. It doesn't follow PEP8. In particular, "Avoid extraneous whitespace [...] immediately inside parentheses."

  2. No docstrings.

  3. It makes more sense to use True and False for a binary condition than to use None and 1.

  4. It's not necessary to append the class name to all the methods. In a class called Image, the methods should just be called load and display, not load_image and display_image.

  5. You should use new-style classes (inheriting from object) to make your code portable between Python 2 and 3.

share|improve this answer
    
I think it can make sense to have separate Image and ImageProxy objects (though a better name might be something like LazyImage), because of separation of concerns. And even better (assuming the added complexity would be worth it) would be some sort of generic LazyLoadingProxy class, which could work with Images and also other objects. –  svick Jan 22 '13 at 14:08
1  
It's possible, but we'd only know in the context of a whole application: if there really are several different types of assets that need to be lazily loaded, then it might make sense to have a class to handle the lazy loading, but it might still be simpler to implement that class as a mixin rather than a proxy. A proxy object is usually a last resort when you don't control the code for the class you are proxying. –  Gareth Rees Jan 22 '13 at 14:17
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.