Making all members of pimpl classes public

Discussion in 'C++' started by Asfand Yar Qazi, Mar 1, 2006.

  1. Hi,

    I'm creating a library where several classes are intertwined rather tightly.
    I'm thinking of making them all use pimpls, so that these circular
    dependancies can be avoided easily, and I'm thinking of making all these pimpl
    class declarations public. Reasoning is that since only the code within the
    ..cc file will need to ever access them, why protect them in ways that would
    make access to them more difficult and obfuscated?

    What are your thoughts on this?

    --
    To reply, take of all ZIGs !!

    Alternative email address: IG
     
    Asfand Yar Qazi, Mar 1, 2006
    #1
    1. Advertising

  2. Asfand Yar Qazi

    Phlip Guest

    Asfand Yar Qazi wrote:

    > I'm creating a library where several classes are intertwined rather
    > tightly. I'm thinking of making them all use pimpls, so that these
    > circular dependancies can be avoided easily


    Use Pimpls in an emergency, as a retrofit to a bad design.

    If you are planning a design, write lots of unit tests, and read the
    Refactoring books to help keep the design clean as it grows.

    Then research the Dependency Inversion Principle, and use that to put in
    just a few abstract interfaces. The necessities of your simple test cases
    will force many of these interfaces to exist.

    > and I'm thinking of making all these pimpl class declarations public.
    > Reasoning is that since only the code within the .cc file will need to
    > ever access them, why protect them in ways that would make access to them
    > more difficult and obfuscated?


    The rule is not "make all data members private". The rule is "don't make
    primitive things globally public".

    You should prefer to make everything as private as possible. Things inside
    implementation files (".cc") are already private, so making them private: is
    irrelevant.

    A .cc file that mostly deals with its own contents, and has very few
    dependencies out to other .cc files, is "coherent". The more coherent your
    ..cc file, the more useful and mobile it is to other .cc files. You can
    change it easily without affecting other .cc files.

    --
    Phlip
    http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
     
    Phlip, Mar 1, 2006
    #2
    1. Advertising

  3. Phlip wrote:
    > Asfand Yar Qazi wrote:
    >
    >
    >>I'm creating a library where several classes are intertwined rather
    >>tightly. I'm thinking of making them all use pimpls, so that these
    >>circular dependancies can be avoided easily

    >
    >
    > Use Pimpls in an emergency, as a retrofit to a bad design.
    >
    > If you are planning a design, write lots of unit tests, and read the
    > Refactoring books to help keep the design clean as it grows.
    >
    > Then research the Dependency Inversion Principle, and use that to put in
    > just a few abstract interfaces. The necessities of your simple test cases
    > will force many of these interfaces to exist.
    >
    >


    I don't think I agree with that - the pimpl idiom easily separates interface
    from implementation. It allows me to place just the interface in the .hh
    file, without reference to a single data member. It easily allows
    copy-on-write behaviour, and allows the interface file to remain unchanged
    even if things underneath change radically.

    Or is that what you meant?

    --
    To reply, take of all ZIGs !!

    Alternative email address: IG
     
    Asfand Yar Qazi, Mar 2, 2006
    #3
  4. Asfand Yar Qazi wrote:

    >
    > I don't think I agree with that - the pimpl idiom easily separates
    > interface from implementation.


    I guess you don't need pimpl to separate interface from implementation,
    of course, if you're not making template classes.
     
    Aleksander Beluga, Mar 2, 2006
    #4
  5. Asfand Yar Qazi

    red floyd Guest

    Asfand Yar Qazi wrote:
    > Phlip wrote:
    >> Asfand Yar Qazi wrote:
    >>
    >>
    >>> I'm creating a library where several classes are intertwined rather
    >>> tightly. I'm thinking of making them all use pimpls, so that these
    >>> circular dependancies can be avoided easily

    >>
    >>
    >> Use Pimpls in an emergency, as a retrofit to a bad design.
    >>
    >> If you are planning a design, write lots of unit tests, and read the
    >> Refactoring books to help keep the design clean as it grows.
    >>
    >> Then research the Dependency Inversion Principle, and use that to put
    >> in just a few abstract interfaces. The necessities of your simple test
    >> cases will force many of these interfaces to exist.

    >

    Pimpl is one of the standard patterns in GoF, although they call it the
    "Bridge" pattern.
     
    red floyd, Mar 2, 2006
    #5
  6. Asfand Yar Qazi

    werasm Guest

    Phlip wrote:
    > Asfand Yar Qazi wrote:


    > Use Pimpls in an emergency, as a retrofit to a bad design.


    This seems naive to me. Pimpls used in conjunction with the Bridge
    pattern are often required for good "dynamic" design. It is certainly
    not only for emergency, or as result of refactoring.

    > If you are planning a design, write lots of unit tests, and read the
    > Refactoring books to help keep the design clean as it grows.


    Specify your interface...

    > Then research the Dependency Inversion Principle, and use that to put in
    > just a few abstract interfaces. The necessities of your simple test cases
    > will force many of these interfaces to exist.


    Hmmm, you are still required to construct your abstract classes
    somewhere. I prefer making my clients dependent on non-abstract
    classes. My pimpls are the abstract classes. Therefore:

    Interface <non-abstract> has a pimpl <abstract>. The Pimpl (or bridge)
    is then provided by a factory. The client has not need to provide, or
    specify the concrete class.

    > You should prefer to make everything as private as possible. Things inside
    > implementation files (".cc") are already private, so making them private: is
    > irrelevant.


    Yes, half true. What about the library implementor. Sometimes common
    behaviour (not data) can be captured in the base class (template method
    pattern). private methods are then called from the public method. On
    the other hand, if this is not the case, and pimpl is implemented
    correctly (i.e. it is abstract), then it only has methods callable from
    the interface, which means there is not need to make anything private.

    The other reason for using pimpl (compiler firewall) often has the
    entire Impl class <then non-abstract> in the cpp file. In this case it
    is fine to make everything public, as the Impl is often a privately
    nested class, which makes it inaccessible to other classes anyway.

    Regards,

    Werner
     
    werasm, Mar 2, 2006
    #6
  7. In message <>, Aleksander
    Beluga <> writes
    >Asfand Yar Qazi wrote:
    >
    >> I don't think I agree with that - the pimpl idiom easily separates
    >>interface from implementation.

    >
    >I guess you don't need pimpl to separate interface from implementation,
    >of course, if you're not making template classes.
    >

    Sometimes you do. Private declarations are conceptually part of the
    implementation, not the interface, but the language syntax requires them
    to appear in the interface definition. This introduces a coupling that's
    not present in the ideal conceptual model which completely separates
    interface and implementation, and this coupling causes recompilation of
    everything that uses the interface whenever the private parts change. A
    major practical reason for using the pimpl idiom is to reduce the
    private part of the public interface to a single pointer, removing that
    syntax-imposed coupling and reducing build times.

    --
    Richard Herring
     
    Richard Herring, Mar 2, 2006
    #7
  8. Asfand Yar Qazi

    Gavin Deane Guest

    Richard Herring wrote:
    > In message <>, Aleksander
    > Beluga <> writes
    > >Asfand Yar Qazi wrote:
    > >
    > >> I don't think I agree with that - the pimpl idiom easily separates
    > >>interface from implementation.

    > >
    > >I guess you don't need pimpl to separate interface from implementation,
    > >of course, if you're not making template classes.
    > >

    > Sometimes you do. Private declarations are conceptually part of the
    > implementation, not the interface, but the language syntax requires them
    > to appear in the interface definition. This introduces a coupling that's
    > not present in the ideal conceptual model which completely separates
    > interface and implementation, and this coupling causes recompilation of
    > everything that uses the interface whenever the private parts change. A
    > major practical reason for using the pimpl idiom is to reduce the
    > private part of the public interface to a single pointer, removing that
    > syntax-imposed coupling and reducing build times.


    It's not only when the private part of the class changes that coupling
    causes recompilation. If the private part of my_class needs something
    defined in "another_header.h" then, without pimpl, "my_class.h" must
    include "another_header.h" or it won't compile (unless a forward
    declaration is sufficient, which isn't always the case).

    Anyone using my_class will obviously include "my_class.h". But then
    they will also be indirectly including "another_header.h" even though
    that header was only of interest to the private part of my_class. If
    "another_header.h" changes, *all* users of my_class will be forced to
    recompile even though they don't care what "another_header.h" is. And
    even if the change doesn't even affect the private part of my_class.

    Use a pimpl in my_class and users of the class are no longer affected
    by any change to the "another_header.h".

    Gavin Deane
     
    Gavin Deane, Mar 2, 2006
    #8
  9. Asfand Yar Qazi

    Phlip Guest

    Asfand Yar Qazi wrote:

    > I don't think I agree with that - the pimpl idiom easily separates
    > interface
    > from implementation. It allows me to place just the interface in the .hh
    > file, without reference to a single data member. It easily allows
    > copy-on-write behaviour, and allows the interface file to remain unchanged
    > even if things underneath change radically.
    >
    > Or is that what you meant?


    Pimpl solves an emergency situation. If you have Foo.h, and it includes
    many .h files, and it declares a huge class Foo, and if everyone uses Foo,
    then each time you change any of those .h files, everything must recompile,
    even things that do not directly use the changed things in those .h files.

    The fastest and easiest fix replaces each .h file with a forward declare of
    the class it defines, and replaces all of Foo's data members with a single,
    bald pointer to FooImpl. Only define FooImpl in Foo.cpp, and forward every
    method to it.

    This is a ruthlessly effective fix because you needn't redesign Foo, or any
    of its users, or any of its servants, to push in the fix. You only need a
    few copies and pastes.

    Now, in terms of the emergency, ask why Foo.h got so burdensome. Why do so
    many things use it? Do they do too much? Why does it use so many servants?
    Do they do too little?

    If a class is well-designed, it will achieve many of Pimpl's benefits,
    possibly including a bald pointer to an implementation object in a .cc
    file. C++ works because the techniques that logically decouple modules
    parallel the techniques that physically decouple them.

    Ideally, if Foo is very important, its clients should access it through an
    abstract interface; call this FooInf (but real code should think of a
    better name). So now the clients cannot create a Foo because they are not
    aware of it. If they want one, they must get it from a Foo Factory of some
    kinds. This technique, "Construction Encapsulation", fully insulates Foo
    clients from the concrete Foo, and its data members, just as rigorously as
    the FooImpl would have. But the entire design is now more flexible.

    --
    Phlip
    http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
     
    Phlip, Mar 2, 2006
    #9
  10. Asfand Yar Qazi

    werasm Guest

    > So now the clients cannot create a Foo because they are not
    > aware of it. If they want one, they must get it from a Foo Factory of some
    > kinds. This technique, "Construction Encapsulation", fully insulates Foo
    > clients from the concrete Foo, and its data members, just as rigorously as
    > the FooImpl would have. But the entire design is now more flexible.


    I mostly agree with you. I have mention this though:

    I have seen applications where factories became the most used
    (included) classes in an application. Every time new products came
    along, the factory interface changed and everything needed to be
    recompiled - especially because the client was made dependent on the
    factory interface. Therefore, if you are implementing a library, hide
    the factory interface from the client. This can be done very
    conveniently by making the client dependent on a non-abstract
    interfaces that makes use of the pimpl (being abstract), who's concrete
    instance is provided by an opaque (not-client-visible) factory.

    Regards,

    Werner

    >
    > --
    > Phlip
    > http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
     
    werasm, Mar 5, 2006
    #10
  11. Asfand Yar Qazi

    Ben Pope Guest

    werasm wrote:
    >> So now the clients cannot create a Foo because they are not
    >> aware of it. If they want one, they must get it from a Foo Factory of some
    >> kinds. This technique, "Construction Encapsulation", fully insulates Foo
    >> clients from the concrete Foo, and its data members, just as rigorously as
    >> the FooImpl would have. But the entire design is now more flexible.

    >
    > I mostly agree with you. I have mention this though:
    >
    > I have seen applications where factories became the most used
    > (included) classes in an application. Every time new products came
    > along, the factory interface changed and everything needed to be
    > recompiled - especially because the client was made dependent on the
    > factory interface. Therefore, if you are implementing a library, hide
    > the factory interface from the client. This can be done very
    > conveniently by making the client dependent on a non-abstract
    > interfaces that makes use of the pimpl (being abstract), who's concrete
    > instance is provided by an opaque (not-client-visible) factory.


    The whole point of a factory is to insulate users from "new products".
    Sounds like a bad implementation of a factory to me.

    Why does the interface of a factory have to change for each new product?
    Why does the implementation even need to change? The whole point is
    to push the management of ids and creating functions into the new
    products, and away from centralised management.

    Ben Pope
    --
    I'm not just a number. To many, I'm known as a string...
     
    Ben Pope, Mar 5, 2006
    #11
  12. Asfand Yar Qazi

    Phlip Guest

    werasm wrote:

    > I have seen applications where factories became the most used
    > (included) classes in an application.


    For every pattern in the book /Design Patterns/, there is a big legacy app
    out there written by those who didn't "get" some pattern, and abused it all
    over the place.

    There are probably more than three such abuse examples per pattern, these
    days. ;-)

    > Every time new products came
    > along, the factory interface changed and everything needed to be
    > recompiled - especially because the client was made dependent on the
    > factory interface. Therefore, if you are implementing a library, hide
    > the factory interface from the client. This can be done very
    > conveniently by making the client dependent on a non-abstract
    > interfaces that makes use of the pimpl (being abstract), who's concrete
    > instance is provided by an opaque (not-client-visible) factory.


    Right - you describe the emergency situation calling for a Pimpl fix.

    The design violated "Construction Encapsulation". (Lots of unit tests would
    have helped fix that, because test cases often construct fake objects and
    pass them to the tested objects, who should not create their own objects.)

    In a typical good design, data enters from the outside world as numbers, and
    then factory-like things convert these to objects. Inside this "boundary
    layer", objects just mix it up, constructing very few new objects, and
    passing references around to each others' interfaces.

    Ordinarily, too many switch statements are a design smell. In such
    well-factored code, the joke is "too many 'if' statements are a design
    smell".

    --
    Phlip
    http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
     
    Phlip, Mar 5, 2006
    #12
  13. Asfand Yar Qazi

    werasm Guest

    > The whole point of a factory is to insulate users from "new products".
    > Sounds like a bad implementation of a factory to me.


    Ok, this may be a naive implementation, but more or less iaw. GOF. I
    haven't got the book here, so bare with me (I'll be extra brief):

    struct Factory
    {
    X* makeX() const;
    Y* makeY() const;
    Z* makeZ() const;
    };

    Ok, X, Y and Z is of course ABC's, right? therefore adding a new
    implementation is fine - this won't cause the client to recompile. But
    what about adding a new ABC?

    struct Factory
    {
    W* makeW const;
    X* makeX() const;
    Y* makeY() const;
    Z* makeZ() const;
    };

    Now interface of this factory has changed. All clients of Factory
    requires recompilation, despite them not making use of W. How do you
    overcome this? I'd love to see - always nice to be less naive
    eventually...

    Regards,

    Werner
     
    werasm, Mar 5, 2006
    #13
  14. Asfand Yar Qazi

    werasm Guest


    > The design violated "Construction Encapsulation".


    Not exactly sure what you mean here. Give me an example of a factory
    not violating "Construction Encapsulation".

    > (Lots of unit tests would
    > have helped fix that, because test cases often construct fake objects and
    > pass them to the tested objects, who should not create their own objects.)


    I may have a vague idea, but I'm not sure I see your point here too.
    Small example please.

    > In a typical good design, data enters from the outside world as numbers, and
    > then factory-like things convert these to objects.


    Factory like things? Does this mean the inner objects use (are exposed
    to the interface) of these factory like things. How do these factory
    like things look. Are they very concise - only responsible for creating
    one type - ever, so that their interface can never change? If yes,
    good. If no, problem.

    > Ordinarily, too many switch statements are a design smell. In such
    > well-factored code, the joke is "too many 'if' statements are a design
    > smell".


    No ifs and no switches sound good, yes. Thats what I prefer.

    Werner
     
    werasm, Mar 5, 2006
    #14
  15. Asfand Yar Qazi

    Phlip Guest

    werasm wrote:

    >> The design violated "Construction Encapsulation".

    >
    > Not exactly sure what you mean here. Give me an example of a factory
    > not violating "Construction Encapsulation".


    This is the answer to your other post:

    > struct Factory
    > {
    > X* makeX() const;
    > Y* makeY() const;
    > Z* makeZ() const;
    > };


    That's not a factory, because calling aFactory.makeX() is the moral
    equivalent of calling new X(). It's just as coupled.

    This is a factory:

    struct Factory
    {
    ABC* make(int type) const;
    };

    It essentially converts a type to a base class pointer. The calling code
    never needs to know the class, only the type code.

    Now this is unencapsulated construction:

    void Foo()
    {
    ABC*p = new X;
    ...
    }

    This is partly encapsulated construction:

    void Foo(int what)
    {
    ABC*p = someFactory.make(what);
    ...
    }

    And this is fully encapsulated construction:

    void Foo(ABC & thing)
    {
    ...
    }

    That's what it means - the clients of objects are not responsible for
    creating their servant objects. They are only responsible for using them,
    and Something Else creates them.

    >> (Lots of unit tests would
    >> have helped fix that, because test cases often construct fake objects and
    >> pass them to the tested objects, who should not create their own
    >> objects.)

    >
    > I may have a vague idea, but I'm not sure I see your point here too.
    > Small example please.


    Suppose the code has three derived classes of ABC. Your X, Y, and Z.

    Now suppose I write a unit test that creates a fourth derivative - MockABC.
    It overrides ABC methods and puts test-ready things in them.

    Which of my Foos is easiest to test? Which one is the easiest to pass a
    MockABC into?

    Now suppose I design my program by writing such tests. The easier classes
    are to test, the more decoupled they become.

    A test case:

    TEST_(TestSuite, doFooThing)
    {
    MockABC aMock;
    Foo(aMock);
    CHECK_EQUAL(42, aMock.fooCalledMeWith());
    }

    > Factory like things? Does this mean the inner objects use (are exposed
    > to the interface) of these factory like things. How do these factory
    > like things look. Are they very concise - only responsible for creating
    > one type - ever, so that their interface can never change? If yes,
    > good. If no, problem.


    They look like simple factories. But you don't happen to call them that, or
    "implement the Class Factory Pattern from the book Design Patterns". They
    might look like the prototype pattern.

    Consider this method:

    void Foo(ABC & thing);

    It has no Factory inside it. Something created the right Thing for it. It
    doesn't care what.

    Factory is a nice pattern, but you could conceivably write a program that
    never couples its constructors to its calling code, yet has no recognizable
    Factory in it.

    --
    Phlip
    http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
     
    Phlip, Mar 5, 2006
    #15
  16. Asfand Yar Qazi

    Ben Pope Guest

    werasm wrote:
    >> The whole point of a factory is to insulate users from "new products".
    >> Sounds like a bad implementation of a factory to me.

    >
    > Ok, this may be a naive implementation, but more or less iaw. GOF. I
    > haven't got the book here, so bare with me (I'll be extra brief):
    >
    > struct Factory
    > {
    > X* makeX() const;
    > Y* makeY() const;
    > Z* makeZ() const;
    > };
    >
    > Ok, X, Y and Z is of course ABC's, right? therefore adding a new
    > implementation is fine - this won't cause the client to recompile. But
    > what about adding a new ABC?
    >
    > struct Factory
    > {
    > W* makeW const;
    > X* makeX() const;
    > Y* makeY() const;
    > Z* makeZ() const;
    > };
    >
    > Now interface of this factory has changed. All clients of Factory
    > requires recompilation, despite them not making use of W. How do you
    > overcome this? I'd love to see - always nice to be less naive
    > eventually...


    Yep, I see your problem. You still need need to know your type at
    runtime in order to instantiate it. You need to separate your type
    information from your creation.

    I don't have time to check this:


    #include <string>
    #include <map>

    struct base{
    virtual ~base() {}
    };

    struct derived : base {};

    base* createBase() {
    return new base;
    }
    base* createDerived() {
    return new derived;
    }

    typedef base* (*creator)();

    class Factory {
    public:
    void Register(std::string id, creator func) {
    factory_[id] = func;
    }
    base* create(std::string id) {
    return factory_[id]();
    }
    private:
    std::map<std::string, creator> factory_;
    };

    int main() {
    Factory factory;
    factory.Register(std::string("base"), &createBase);
    factory.Register(std::string("derived"), &createBase);

    base* b = factory.create(std::string("base"));
    return 0;
    }


    If you don't get it, I'm sure others can explain.

    This is also covered in Modern C++ design (infinitely more elegant than
    my example!)

    Ben Pope
    --
    I'm not just a number. To many, I'm known as a string...
     
    Ben Pope, Mar 5, 2006
    #16
  17. red floyd wrote:
    > Asfand Yar Qazi wrote:
    >> Phlip wrote:


    >>> Then research the Dependency Inversion Principle, and use that to put
    >>> in just a few abstract interfaces. The necessities of your simple
    >>> test cases will force many of these interfaces to exist.

    >>

    > Pimpl is one of the standard patterns in GoF, although they call it the
    > "Bridge" pattern.
    >


    I believe that pImpl idiom is only C++ solution while DPs are usually
    cross-language. Also, I believe that pImpl is a specific case of another
    DP called Facade.
     
    Aleksander Beluga, Mar 5, 2006
    #17
  18. Asfand Yar Qazi

    Phlip Guest

    Aleksander Beluga wrote:

    > I believe that pImpl idiom is only C++ solution while DPs are usually
    > cross-language. Also, I believe that pImpl is a specific case of another
    > DP called Facade.


    DPs are logical design. Pimpl is physical design. That means you could take
    the pimpl out, and all the client classes would behave the same. Except
    they'd take longer to compile.

    A refactor to a better design, such as via the Dependency Inversion
    Principle, _should_ simplify all the clients.

    --
    Phlip
    http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
     
    Phlip, Mar 5, 2006
    #18
  19. * Phlip:
    > Aleksander Beluga wrote:
    >
    >> I believe that pImpl idiom is only C++ solution while DPs are usually
    >> cross-language. Also, I believe that pImpl is a specific case of another
    >> DP called Facade.

    >
    > DPs are logical design. Pimpl is physical design. That means you could take
    > the pimpl out, and all the client classes would behave the same. Except
    > they'd take longer to compile.


    Nope.

    pimpl removes dependencies on header files, some of which are usually
    ill-behaved, and anyway, which introduce definitions not present with pimpl.


    > A refactor to a better design, such as via the Dependency Inversion
    > Principle, _should_ simplify all the clients.


    On the contrary. ;-)


    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Mar 6, 2006
    #19
  20. Asfand Yar Qazi

    Phlip Guest

    Alf P. Steinbach wrote:

    >> DPs are logical design. Pimpl is physical design. That means you could
    >> take
    >> the pimpl out, and all the client classes would behave the same. Except
    >> they'd take longer to compile.

    >
    > Nope.
    >
    > pimpl removes dependencies on header files, some of which are usually
    > ill-behaved, and anyway, which introduce definitions not present with
    > pimpl.


    DPs are logical design. Pimpl is physical design. That means you could take
    the pimpl out, and all the client classes would have exactly the same design
    and contents. Except they'd take longer to compile, and would be exposed to
    all the stray identifiers in the excess headers, that could change their own
    identifier's meanings in subtle ways, if your C++ codebase was already
    circling the drain anyway.

    >> A refactor to a better design, such as via the Dependency Inversion
    >> Principle, _should_ simplify all the clients.

    >
    > On the contrary. ;-)


    Else why put it in?

    --
    Phlip
    http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
     
    Phlip, Mar 6, 2006
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Charles A. Lackman
    Replies:
    1
    Views:
    1,376
    smith
    Dec 8, 2004
  2. SpamProof
    Replies:
    0
    Views:
    583
    SpamProof
    Oct 21, 2003
  3. lovecreatesbeauty
    Replies:
    1
    Views:
    320
    Karl Heinz Buchegger
    Apr 14, 2005
  4. Daz
    Replies:
    5
    Views:
    583
  5. Graham Reitz
    Replies:
    2
    Views:
    552
    Brian Szmyd
    Apr 17, 2008
Loading...

Share This Page