Re: [boost] Please look at my suggested pimpl library in the file vault.
I've uploaded an updated version ...
Pavel Vozenilek wrote ... I do have a few. /Pavel
________________________________________________ 1. You should not use double underscores (and neither leading single underscore). These are reserved for standard.
It's corrected.
________________________________________________ 2. It is better when member functions are all lowercase (like elsewhere in Boost).
It's corrected.
________________________________________________ 3. pimpl.hpp:
The base::_Get() should have BOOST_ASSERT() before dereferencing.
Possibly this function could be omitted altogether since it is always overriden.
It's corrected.
-----------
_Destructor should be named destroy() to evoke action.
Better yet, move it into real destructor.
It's corrected.
-----------
late_creation policy may nam named "manual_creation" or so.
The Set() should destroy previous instance if non-NULL.
The _Get() should contain BOOST_ASSERT() or throw if NULL.
It's corrected.
---------------
Closing bracket for namespace should nopt have ";" but should have comment like // namespace pimpl_policies
It's corrected.
----------------
operator=() does shallow copy of dynamically allocated value. Is this really what ypu want?
It's corrected.
________________________________________________ 4. Compiling with Intel C++ 7.0 for Windows I get error:
testsuite.cpp C:\work\reviews\pimpl\testsuite.cpp(54): error: no suitable user-defined conversion from "boost::pimpl<CMyValues1,
boost::pimpl_policies::base>" to "const boost::pimpl<CMyValues2, boost::pimpl_policies::late_creation>" exists test.m_ValuesWithLazyCreator2 = test.m_ValuesWithDefaultConstructor;
I've now tested with Intel C++ 9.0 windows and it works fine. How can I get a copy of this old version?
________________________________________________ 5. The pimpl doesn't show how I work with member functions. This is the most important part of the idiom and If there's way to make the tedious work any simpler I am all for it.
You just work with the pimpl's -> operator. That will give you access to the pimpl's member functions. The new 'testsuite.cpp' shows its use. And thanks for your corrections, I highly appreciate them. Regards, Asger Mangaard
From: "Asger Mangaard" <tmb@tmbproductions.com>
Pavel Vozenilek wrote ...
Closing bracket for namespace should nopt have ";" but should have comment like // namespace pimpl_policies
It's corrected.
"Should" is a strong word there. I dislike such comments because they too easily get out of sync with the start of the block. Some like that style, some don't.
----------------
operator=() does shallow copy of dynamically allocated value. Is this really what ypu want?
It's corrected.
I just downloaded the library and it still does a shallow copy. If you intend something else, then you need to rethink how you handle copying. ___________________________________ I have some of my own comments now. * Why the pimpl_policies namespace? I think you should have a boost::pimpls namespace. In that you can have the pimpl class and the supplied policies. * The base class should be in namespace detail. The name suggests that this type isn't one clients of the library should be using. However, the behavior of the class suggests that you really want it to serve two purposes (also evidenced by its being the default pimpl policy type). I suggest a new type, derived from base, called, say, "default_construction" that can be the default policy. That class can construct an instance in its default constructor and provide the get() member function you currently have in base. Once you do those things, then base can have a constructor taking a raw pointer to the impl object and you can eliminate the get() member function. At that point, base will have only one purpose instead of the two it now has. * Why is "PimplType" not just "T?" The whole point of the template is to house a pimpl type, so the added verbosity doesn't seem helpful. * base::get() error handling should be configurable or nothing. Throwing an exception isn't always the appropriate action. Some would prefer getting a segfault and a core dump, for example. Either omit the test of m_pPimpl or control it via another policy. * You m_pPimpl data member name is quite odd. The reason this is the "pimpl" idiom is because of the common "p" prefix for pointers to an implementation object. That leads to "pimpl_," "m_pImpl," "_pimpl," and other variations, depending upon your naming scheme. Using both the "p" prefix and the "P" in the data member name is redundant. My preference is "pimpl_," but "m_pImpl" is probably yours. * boost::pimpl_policies::base::base() misuses the new operator. "new PimplType()" is incorrect. You want "new PimplType." * boost::pimpl_policies::base::~base() should be non-virtual. No one should be creating polymorhpic instances of the pimpl. * Your explanation of the mutability of m_pPimpl is redundant. That's the reason mutable exists. You might want to be more specific as to the function that uses it or just remove the comment. * Since lazy_creation and manual_creation derive from base, you'll cause warnings about hiding base::get(). base should not have a get() member function. The derived policies should have to implement get(). * I'm not sure whether manual_creation should be a separate policy. Why not have the default policy have a constructor taking a raw pointer, defaulted to a null pointer, plus provide a set() member function? You'd need to implement set() just as it is in manual_creation now. namespace boost { namespace pimpls { template <typename T> struct default_ : base<T> { default_(T * pimpl_i = 0) : pimpl_(pimpl_i) { } T & get() const { return *pimpl_; } void set(T * pimpl_i) { delete pimpl_; pimpl_ = pimpl_i; } }; } } Thus, default_ does what your base and manual_creation policies do in one simple policy. * Don't define empty destructors. They are noise in the code and can actually be a hindrance to optimal code generation. * pimpl's member functions should be inlined. They are all trivial, so you should make them inline. * Consider "rhs" or "other" instead of "comparer." This is simply a stylistic suggestion which isn't terribly important, but your use of "comparer" stood out. First, what you've called "comparer" isn't comparing. Second, it's a rather long name for something so straightforward. -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;
I've uploaded another version with your suggestions. Thanks.
Rob Stewart wrote
----------------
operator=() does shallow copy of dynamically allocated value. Is this really what ypu want?
It's corrected.
I just downloaded the library and it still does a shallow copy. If you intend something else, then you need to rethink how you handle copying.
Could you be more specific? I really don't see where it shallows it.
I think you should have a boost::pimpls namespace. In that you can have the pimpl class and the supplied policies. I renamed the policies namespace to pimpls. I keep the class itself inside the boost namespace to shorten the name, eg. boost:pimpl instead of boost::pimpls::pimpl.
* The base class should be in namespace detail. Corrected.
I suggest a new type, derived from base, called, say, "default_construction" that can be the default policy. Corrected.
* Why is "PimplType" not just "T?" Corrected.
* base::get() error handling should be configurable or nothing. I've removed the exception, and rely on the user to know how to fix a null pointer.
* You m_pPimpl data member name is quite odd. Renamed to m_pImpl
* boost::pimpl_policies::base::base() misuses the new operator. corrected.
* boost::pimpl_policies::base::~base() should be non-virtual. Corrected.
* I'm not sure whether manual_creation should be a separate policy.
Why not have the default policy have a constructor taking a raw pointer, defaulted to a null pointer, plus provide a set() member function? You'd need to implement set() just as it is in manual_creation now.
namespace boost { namespace pimpls { template <typename T> struct default_ : base<T> { default_(T * pimpl_i = 0) : pimpl_(pimpl_i) { } T & get() const { return *pimpl_; } void set(T * pimpl_i) { delete pimpl_; pimpl_ = pimpl_i; } }; } }
Thus, default_ does what your base and manual_creation policies do in one simple policy.
But then you remove the default creation which is one of the advantages of the library.
* Don't define empty destructors.
They are noise in the code and can actually be a hindrance to optimal code generation. Corrected.
* pimpl's member functions should be inlined.
They are all trivial, so you should make them inline. Aren't templates always inlined?
* Consider "rhs" or "other" instead of "comparer." Corrected. I choose rhs.
Thanks for all the suggestions, Asger Mangaard
Asger Mangaard wrote:
* Why is "PimplType" not just "T?"
Corrected.
Why was this considered a problem in the first place? Aren't more descriptive names generally better? The tendency to use single letters for template parameters seems flawed to me.
* I'm not sure whether manual_creation should be a separate policy.
Why not have the default policy have a constructor taking a raw pointer, defaulted to a null pointer, plus provide a set() member function? You'd need to implement set() just as it is in manual_creation now.
namespace boost { namespace pimpls { template <typename T> struct default_ : base<T> { default_(T * pimpl_i = 0) : pimpl_(pimpl_i) { } T & get() const { return *pimpl_; } void set(T * pimpl_i) { delete pimpl_; pimpl_ = pimpl_i; } }; } }
Thus, default_ does what your base and manual_creation policies do in one simple policy.
But then you remove the default creation which is one of the advantages of the library.
Wow, this definitely sounds like it mirrors some of my singleton work. If it interests you, the way I currently handle automatic creation as a policy is by having it define functors which are invoked by the smart pointer constructor and operator ->(). The functors take a pointer to a factory as the only parameter. You can get greedy_creation by making both of these functors create the instance if it does not exist, lazy_creation by having the constructor functor be a no-op and only operator-> creates the instance, and manual_creation by making both of these functors no-ops. The type of factory to use is provided as a completely orthogonal policy which actually manages the semantics of creation and destruction (using new, malloc, an std_allocator, an in place aligned_storage buffer, etc. Unfortunately I have not yet had time to look at the code for your library, so these ideas may or may not apply.
* pimpl's member functions should be inlined.
They are all trivial, so you should make them inline.
Aren't templates always inlined?
Agreed, and besides, isn't it usually better to let the compiler decide what to inline or not inline on its own unless extensive profiling shows that adding the inline keyword really gives you a big performance gain? -Jason
From: Jason Hise <chaos@ezequal.com>
Asger Mangaard wrote:
* Why is "PimplType" not just "T?"
Corrected.
Why was this considered a problem in the first place? Aren't more descriptive names generally better? The tendency to use single letters for template parameters seems flawed to me.
I didn't consider it a problem. I was questioning the value of the longer name. If there is only one parameterizing type for a template, the ubiquitous "T" is well suited. If there is a concept the type must fulfill, then naming it according to the concept is worthwhile. If there are many parameterizing types, longer names can make it easier to distinguish among the types. In this case, the whole purpose of the class template is to wrap one type which fulfills no concept definition, so "T" is perfectly suited. The benefit is more readable code.
Thus, default_ does what your base and manual_creation policies do in one simple policy.
But then you remove the default creation which is one of the advantages of the library.
Wow, this definitely sounds like it mirrors some of my singleton work. If it interests you, the way I currently handle automatic creation as a policy is by having it define functors which are invoked by the smart pointer constructor and operator ->(). The functors take a pointer to a factory as the only parameter. You can get greedy_creation by making
Singletons have to come into being at relatively arbitrary times and be accessible to many clients. Pimpls are used in highly restricted contexts. The approach you're using for Singletons seems like overkill. For Pimpls, it's a simple matter of initializing in a constructor or assigning in some member function. After that, it's all managed for you.
* pimpl's member functions should be inlined.
They are all trivial, so you should make them inline.
Aren't templates always inlined?
Agreed, and besides, isn't it usually better to let the compiler decide
Does that mean you think they are always inlined?
Agreed, and besides, isn't it usually better to let the compiler decide what to inline or not inline on its own unless extensive profiling shows that adding the inline keyword really gives you a big performance gain?
Many compilers don't do it at all unless you suggest it via "inline" or put the code in the class definition. Some will inline things you didn't suggest should be. In this case, the functions are nothing but forwarding functions. Why would you want to pay for an extra function call on those compilers that won't do anything unless you ask for it? -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;
Rob Stewart wrote:
* pimpl's member functions should be inlined.
They are all trivial, so you should make them inline.
Aren't templates always inlined?
Agreed, and besides, isn't it usually better to let the compiler decide what to inline or not inline on its own unless extensive profiling shows that adding the inline keyword really gives you a big performance gain?
Does that mean you think they are always inlined?
Many compilers don't do it at all unless you suggest it via "inline" or put the code in the class definition. Some will inline things you didn't suggest should be.
In this case, the functions are nothing but forwarding functions. Why would you want to pay for an extra function call on those compilers that won't do anything unless you ask for it?
Based on reading item 25 in Herb Sutter's "Exceptional C++ Style", I was under the impression that a decent compiler would easily inline calls like this on its own, without any suggestions from the programmer. But then I haven't directly done any profiling myself to compare how differently modern compilers behave when explicitly provided with the inline keyword, so I could well just be under a mistaken impression. -Jason
From: Jason Hise <chaos@ezequal.com>
Rob Stewart wrote:
Many compilers don't do it at all unless you suggest it via "inline" or put the code in the class definition. Some will inline things you didn't suggest should be.
In this case, the functions are nothing but forwarding functions. Why would you want to pay for an extra function call on those compilers that won't do anything unless you ask for it?
Based on reading item 25 in Herb Sutter's "Exceptional C++ Style", I was under the impression that a decent compiler would easily inline calls like this on its own, without any suggestions from the programmer. But then I haven't directly done any profiling myself to compare how differently modern compilers behave when explicitly provided with the inline keyword, so I could well just be under a mistaken impression.
Not all compilers/linkers do what Herb is talking about. When you are targeting your code to a single platform, you can find that it is better to avoid inlining unless you've profiled. If your function simply calls another function, then inlining is harmless for those platforms that would inline it anyway, and helpful for those that wouldn't. Thus, in writing a library, you can reasonably inline the simplest of functions (provided you know about the hidden aspects that may be lurking behind the trivial source code). The simplest way to confirm that inlining is helpful is to write and profile test code with and without the inline keyword. That doesn't account for all uses of the code, but it is reasonable. (A Boost config macro indicating how well a platform does at inlining could be used to control whether you write inline or not. When all is said and done, however, the compiler can ignore your request to inline a function anyway.) In the pimpl library case, the pimpl member functions I was referring to were as simple as they get. Now, taking another look, I note that the policies inline some functions that probably shouldn't be inlined. As coded in the version of the library I have, for example, base::get() is much too big to assume inlining is a good idea (it constructs and throws an exception if a condition is satisfied). If you change base::get() to simply return the dereferenced pointer, then it should be inlined. Then, pimpl::operator -() will be an inlined call to base::get(), and base::get() will be inlined to nothing more than a pointer dereference. Thus, using a pimpl will be just the same as using a raw pointer (when using the member selection operator, at least). -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;
I've uploaded an updated version.
Rob Stewart wrote: In the pimpl library case, the pimpl member functions I was referring to were as simple as they get. Now, taking another look, I note that the policies inline some functions that probably shouldn't be inlined. As coded in the version of the library I have, for example, base::get() is much too big to assume inlining is a good idea (it constructs and throws an exception if a condition is satisfied). If you change base::get() to simply return the dereferenced pointer, then it should be inlined. Then, pimpl::operator -() will be an inlined call to base::get(), and base::get() will be inlined to nothing more than a pointer dereference. Thus, using a pimpl will be just the same as using a raw pointer (when using the member selection operator, at least).
I've now inlined all the pimpl functions. Regards, Asger Mangaard
On Thu, 06 Oct 2005 09:31:25 +0100, Asger Mangaard <tmb@tmbproductions.com> wrote:
I've uploaded an updated version.
I've had a look at pimpl.zip in the root of the sandbox just now, and having read the foregoing discussions, I am still confused how this implements a pimpl. According to "Exceptional C++" there are four common pimpl implementation types: " 1. All private data (but not functions) go in the impl 2. Put all private members into impl 3. Put all private and protected members into impl 4. Make pimpl entirely the class that impl would have been, and write pimpl as only the public interface made up entirely of simple forwarding functions (a handle/body variant)" (It states that number 3 is "wrong".) In variants 1 and 2 pimpl has the public and protected methods in it and their implementation. In variant 4 pimpl has forwarding functions. How do I add member functions and their implementation to your pimpl class? Richard
Richard wrote:
According to "Exceptional C++" there are four common pimpl implementation types: " 1. All private data (but not functions) go in the impl 2. Put all private members into impl 3. Put all private and protected members into impl 4. Make pimpl entirely the class that impl would have been, and write pimpl as only the public interface made up entirely of simple forwarding functions (a handle/body variant)"
(It states that number 3 is "wrong".)
In variants 1 and 2 pimpl has the public and protected methods in it and their implementation. In variant 4 pimpl has forwarding functions. How do I add member functions and their implementation to your pimpl class?
What is discussed in exceptional C++ is the use of the pimpl idiom. The library does not implement any of these types, you do. The library just eases the underlying work with memory management, etc. Eg. A gameplayer class. Type 1: --------- .hpp: struct GamePlayer { void IncreaseLives(); void MoveTo( Vector3 _newPosition); private: boost::pimpl<struct GamePlayerValues> m_Values; }; .cpp struct GamePlayerValues { CMyIntegerType m_Lives; Vector3 m_Position; }; void GamePlayer::IncreaseLives() { m_Values->m_Lives ++; } void GamePlayer::MoveTo( Vector3 _newPosition) { m_Values->m_Position = _newPosition; } etc. In the gameplayer class we hide all our 'private' values. So when someone includes the hpp file they don't see that our 'lives' are stored in a special integer class, CMyIntegerType. This has several advantages: * When 'CMyIntegerType' changes only the cpp file needs recompiling thus potentially reducing compile times greatly. * You completely hide the content of 'GamePlayerValues' from all other than the cpp file. In case it's secret :) Also, I guess type 3 is 'wrong' because protected values should be visible to derived classes, and with the pimpl idiom you hide those values like explained above. Regards, Asger Mangaard
On 10/7/05, Asger Mangaard <tmb@tmbproductions.com> wrote:
What is discussed in exceptional C++ is the use of the pimpl idiom. The library does not implement any of these types, you do. The library just eases the underlying work with memory management, etc.
Ergo I restate my case that this library should NOT be called Pimpl. Maybe PimplHolder or something like that, but calling it Pimpl confuses at least two people :-) -- Caleb Epstein caleb dot epstein at gmail dot com
From: Caleb Epstein <caleb.epstein@gmail.com>
Ergo I restate my case that this library should NOT be called Pimpl. Maybe PimplHolder or something like that, but calling it Pimpl confuses at least two people :-)
I was never confused as to whether this library would magically implement the impl class, though I think Asger may have flirted with the idea of forwarding functions. Nevertheless, I can see calling the class pimpl_ptr, if that suffix would clarify matters for you. That also highlights what Arkadiy--if I remember correctly-- commented on: this is just a special case for a PBSP, with smart_ptr (in the Review queue) being the likely candidate. -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;
On 10/7/05, Rob Stewart <stewart@sig.com> wrote: pimpl_ptr. Thats sounds like a perfect name. That also highlights what Arkadiy--if I remember correctly--
commented on: this is just a special case for a PBSP, with smart_ptr (in the Review queue) being the likely candidate.
I think it was Gennadiy. And I think he's right. -- Caleb Epstein caleb dot epstein at gmail dot com
Not impl_ptr? Caleb Epstein wrote:
On 10/7/05, Rob Stewart <stewart@sig.com> wrote:
pimpl_ptr.
Thats sounds like a perfect name.
That also highlights what Arkadiy--if I remember correctly--
commented on: this is just a special case for a PBSP, with smart_ptr (in the Review queue) being the likely candidate.
I think it was Gennadiy. And I think he's right. -- Caleb Epstein caleb dot epstein at gmail dot com _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
From: Deane Yang <deane_yang@yahoo.com>
Not impl_ptr?
That's too generic; it doesn't evoke "pimpl idiom" to me. -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;
Rob Stewart wrote:
From: Caleb Epstein <caleb.epstein@gmail.com>
Ergo I restate my case that this library should NOT be called Pimpl. Maybe PimplHolder or something like that, but calling it Pimpl confuses at least two people :-)
I was never confused as to whether this library would magically implement the impl class, though I think Asger may have flirted with the idea of forwarding functions. Nevertheless, I can see calling the class pimpl_ptr, if that suffix would clarify matters for you.
Let's not forget that the first P in pimpl stands for 'pointer'. :-)
From: "Peter Dimov" <pdimov@mmltd.net>
Rob Stewart wrote:
From: Caleb Epstein <caleb.epstein@gmail.com>
Ergo I restate my case that this library should NOT be called Pimpl. Maybe PimplHolder or something like that, but calling it Pimpl confuses at least two people :-)
I was never confused as to whether this library would magically implement the impl class, though I think Asger may have flirted with the idea of forwarding functions. Nevertheless, I can see calling the class pimpl_ptr, if that suffix would clarify matters for you.
Let's not forget that the first P in pimpl stands for 'pointer'. :-)
I raised that point previously, but that didn't seem to help. -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;
From: =?ISO-8859-1?Q?Michel_Andr=E9?= <michel.andre@swipnet.se>
Peter Dimov wrote:
Let's not forget that the first P in pimpl stands for 'pointer'. :-)
At occassions I have seen it described and denoted as p(rivate)impl(ementation).
According to Sutter (Exceptional C++, 4th printing, p103, footnote 1): The eponymous pimpl_ was actually coined several years ago by Jeff Sumner (chief programmer at PeerDirect), due in equal parts to a penchant for Hungarian-style "p" prefixes for pointer variables and an occasional taste for horrid puns. Thus, "p" stands for "pointer." -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;
"Rob Stewart" <stewart@sig.com> wrote
That also highlights what Arkadiy--if I remember correctly-- commented on: this is just a special case for a PBSP, with smart_ptr (in the Review queue) being the likely candidate.
I think it was Gennadiy Rozental. Regards, Arkadiy
Jason Hise <chaos@ezequal.com> writes:
Aren't templates always inlined?
Agreed, and besides, isn't it usually better to let the compiler decide what to inline or not inline on its own unless extensive profiling shows that adding the inline keyword really gives you a big performance gain?
Template functions are NOT always inlined. Like inline functions (which can be defined in a header), template function definitions may also be defined in a header. That does not, however, make them inlined. If you would like it to be inlined, you still should use inline keyword. The compiler is always free to reject your suggestion and not inline a function, but if you don't make the request, the compiler probably won't inline it. (Only aggressive optimizers might inline functions that you didn't request to have inlined, but that's outside the spec.) Also, think about exported templates. If you try to declare a template function to be both exported and inlined, the standard dictates that the function will be treated as if it were just declared to be inlined. Similarly, in 14.7.3 p14, it states that explicit template specializations are only inline if they're explicitly declared to be inline. -- Chris
From: "Asger Mangaard" <tmb@tmbproductions.com>
Rob Stewart wrote
I just downloaded the library and it still does a shallow copy. If you intend something else, then you need to rethink how you handle copying.
Could you be more specific? I really don't see where it shallows it.
Oops. I looked again. You're calling get() and that returns a reference. I had std::auto_ptr<T>::get(), for example, in mind when I saw that. You're fine, although *this = rhs is unusual for a copy constructor.
I think you should have a boost::pimpls namespace. In that you can have the pimpl class and the supplied policies. I renamed the policies namespace to pimpls. I keep the class itself inside the boost namespace to shorten the name, eg. boost:pimpl instead of boost::pimpls::pimpl.
If someone wants it shortened, they can use a using declaration. I'm not sure this warrants being directly in the boost namespace.
* I'm not sure whether manual_creation should be a separate policy.
Why not have the default policy have a constructor taking a raw pointer, defaulted to a null pointer, plus provide a set() member function? You'd need to implement set() just as it is in manual_creation now.
But then you remove the default creation which is one of the advantages of the library.
Ummm, yeah. Sorry. I was only paying attention to one side of that coin, it seems. It occurs to me that the creation policy names could be shortened to "lazy," "manual," and "default_."
* pimpl's member functions should be inlined.
They are all trivial, so you should make them inline. Aren't templates always inlined?
Nope. They work just like non-templated (member) functions. -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;
I've just had a look at the files in the sandbox from 10th October and have a couple of comments. It would be useful if your test programs actually tested that this does implement the compiler firewall feature for all construction policies, I think it only does for the CGamePlayer class and default construction. Using pointers and incomplete types leads to me ask how this library handles deletion of incomplete types. Your documentation suggests that CGamePlayer does not need a destructor, but example.cpp in the zip file includes a destructor in the cpp file. Not having a destructor in the cpp file suggests that you'll get the incorrect destructor called? Should there be a check in there, like scoped_ptr has a static assert? Or is this taken care of in the library? Richard
participants (10)
-
Arkadiy Vertleyb -
Asger Mangaard -
Caleb Epstein -
Chris Uzdavinis -
Deane Yang -
Jason Hise -
Michel André -
Peter Dimov -
Richard Jennings -
Rob Stewart