[design] policy based. Was: [XInt] Review
2011/3/10 Chad Nelson <chad.thecomfychair@gmail.com>:
On Wed, 9 Mar 2011 19:11:31 +0100 Joachim Faulhaber <afojgo@googlemail.com> wrote:
This is my review on the XInt library. [...]
Thank you for the review, and the comments.
Thank you for your replies and questions [...]
2.3 Policy based design =======================
I think policy based design is a very good and powerful technique that is specifically useful to separate implementation variants of class templates in clear and flexible ways. Unfortunately I can not see that you are actually using policy based design here. Because your design lacks typical policy classes that encapsulate implementation variants in a way that reduces codereplication and enhances flexibility. [...]
As I'm not sure what kind of implementation you're thinking of, I can't respond to this. So far as I can tell, my design is doing what you describe for policy-based design.
There is a great deal of code replication in the current implementation of xint::integer_t. [...] This skeleton (and some variations of it) occurs over and over again only to propagate the call of the function to the implementing class at positions #1 and #2.
integer_t handles some options, and passes the data to the implementing class for others. It was the best solution I could find to handle all options.
Trying to be consistent with my own request ...
"At the same time I'd like to call on the boost community to support Chad and to see an Integer library as an opportunity to develop more common understanding how a state of the art generic design looks like in this specific case."
... I invested some work to make my suggestion about good design more concrete. I will give you a small example of a policy based design that is directly related to some problems of code replication and insufficient separation of concerns in your library. At the same time I hope it'll help to understand policy based design, which includes getting a better understanding for myself. The only source of PDB that I've been using so far is Andrei's book from 2001. [Andrei Alexandrescu 2001, Moden C++ Design] But I found the page on wikipedia informative as well. http://en.wikipedia.org/wiki/Policy-based_design Maybe others can contribute more good sources, I did not research. Andrei's book has been really enlightening for me. I've never read a book about programming with so much delight. I can highly recommend it. As for the example. I picked one aspect of your design, the option for nothrow, that controls the handling of nan. Here I've criticized code replication in your code like if (Nothrow) {if (b.is_nan()) ... if (!is_nan()) { ... }} else {...} that reoccurs a lot in the implementation of integer_t. Using policy based design, we try to encapsulate all aspects of the code that relates to the handling of nans in a policy class. So we try to separate the concern "handling nan" from other parts of a class. In this case aint (not xint ;) //========================================================= // Using policies, we want to handle nan in a "nany" or // a "manly" way (manly: ignore it all together) struct nany; // Takes care for the nasty nan struct manly; // No, we don't want this template<class, class> class aint; // a int with nan policy // Meta function for naniness. template<class Type> struct is_nany { static const bool value = false; }; template<class Rep> // a int with nany policy is_nany struct is_nany<aint<Rep, nany> > { static const bool value = true; }; template<class Type> bool is_nan(Type const& value) { // Not perfect ... ok. for the example return value ==( numeric_limits<Type>::has_quiet_NaN ? (numeric_limits<Type>::quiet_NaN)() : (numeric_limits<Type>::max)() ); } // An int class with NaNPolicy: Rep is supposed to be some // integer implementation, including limitless integers // NaNPolicy encapsulates nan-handling. template<class Rep, class NaNPolicy = manly> class aint // no sunshine when she's gone { public: aint(): _value(){} aint(Rep const& val): _value(val){} aint& operator += (aint const& y) { NaNPolicy::plus_assign(_value, y._value); return *this; } Rep value(){ return _value; } private: Rep _value; }; // Finally two different NaN-policy classes struct nany { template<class Rep> inline static void plus_assign(Rep& x, Rep const& y) { if (is_nan(x)) ; else if(is_nan(y)) x = y; else x += y; } }; struct manly { template<class Rep> inline static void plus_assign(Rep& x, Rep const& y) { x += y; } }; BOOST_AUTO_TEST_CASE(nan_policy_study) { // No NaN protection aint<long> ago; // A no throw int with NaN protection aint<short, nany> sun(2); aint<short, nany> shine(7); sun += shine; cout << sun.value() << endl; // I is generic because it is suitable for all kinds of // integer types Rep including say xint::integer_t types aint<xint::integer, nany> ax_i; } //========================================================= 1. Encapsulation: The code necessary for the handling of nans is either encapsulated in the policy class, or it uses some meta code that is globally available. 2. Non intrusive: This code does not require changes in the class Rep, that implements the basic semantics. 3. Generic: So, apart from very little code around the existence and test of the NaN-element, the policy based design works for all integer implementations Rep that are model of a common integer concept, which is implicitly given in this case. 4. Abstraction: Moreover separation of concerns leads to abstraction here. We discover that this kind of "nan closure" not only works for integers but also for floting point numbers or continuous numbers like e.g. boost::rational. Regards, Joachim -- Interval Container Library [Boost.Icl] http://www.joachim-faulhaber.de
On Thu, 10 Mar 2011 23:06:46 +0100 Joachim Faulhaber <afojgo@googlemail.com> wrote:
integer_t handles some options, and passes the data to the implementing class for others. It was the best solution I could find to handle all options.
[...] I invested some work to make my suggestion about good design more concrete. I will give you a small example of a policy based design that is directly related to some problems of code replication and insufficient separation of concerns in your library. At the same time I hope it'll help to understand policy based design, which includes getting a better understanding for myself. [...]
I think I see what you're getting at. Thank you. I'll have to think about that further to see the best way to implement it. -- Chad Nelson Oak Circle Software, Inc. * * *
On 03/11/11 02:20 AM, Chad Nelson wrote:
On Thu, 10 Mar 2011 23:06:46 +0100 Joachim Faulhaber<afojgo@googlemail.com> wrote:
integer_t handles some options, and passes the data to the implementing class for others. It was the best solution I could find to handle all options.
[...] I invested some work to make my suggestion about good design more concrete. I will give you a small example of a policy based design that is directly related to some problems of code replication and insufficient separation of concerns in your library. At the same time I hope it'll help to understand policy based design, which includes getting a better understanding for myself. [...]
I think I see what you're getting at. Thank you. I'll have to think about that further to see the best way to implement it.
As a side-note, in that context, providing an implementation (sometime in the future) similar to Microsoft's SafeInt [1] would be an extremely useful addition to this library. This would be more useful than a simple binary fail-state, IMHO. Best regards Jan [1] http://msdn.microsoft.com/en-us/library/ms972705
On Fri, 11 Mar 2011 10:51:24 +0100 JCK <jan.kleinsorge.bulk@googlemail.com> wrote:
[...] I think I see what you're getting at. Thank you. I'll have to think about that further to see the best way to implement it.
As a side-note, in that context, providing an implementation (sometime in the future) similar to Microsoft's SafeInt [1] would be an extremely useful addition to this library. This would be more useful than a simple binary fail-state, IMHO.
That would be easy to add, interface-wise. Just a "throw on overflow" option, for use with fixed-size integers. I'll put it on the to-do list, but I think it'll be quite a while before I get down to it. -- Chad Nelson Oak Circle Software, Inc. * * *
2011/3/12 Chad Nelson <chad.thecomfychair@gmail.com>:
On Fri, 11 Mar 2011 10:51:24 +0100 JCK <jan.kleinsorge.bulk@googlemail.com> wrote:
[...] I think I see what you're getting at. Thank you. I'll have to think about that further to see the best way to implement it.
As a side-note, in that context, providing an implementation (sometime in the future) similar to Microsoft's SafeInt [1] would be an extremely useful addition to this library. This would be more useful than a simple binary fail-state, IMHO.
That would be easy to add, interface-wise. Just a "throw on overflow" option, for use with fixed-size integers. I'll put it on the to-do list, but I think it'll be quite a while before I get down to it.
With respect to the side note of Jan, I'd like to resume the example of a policy based design. As we are separating concerns and encapsulate them into policy classes, we should be able to code a behavior like the MS SafeInt mentioned by Jan in a policy class. typedef aint<int, MsSafeIntBehavior> aSafeInt; typedef aint<xint::integer_t<options::fixedlenth<512> >, MsSafeIntBehavior> anXSafeInt; Note, that in this type of design: (1) The "policy equipped" class (aint) doesn't have to be changed. (2) The implementing class e.g. xint::integer_t doesn't have to be changed (3) The author of xint, doesn't need to do anything (4) The client developer, that implements a new policy class 'MsSafeIntBehavior' can develop a behavior of her choice. So we have: (1) Separation of concerns/encapsulation (2) Extensibility by user code (3) Abstraction As already mentioned in the first posting in this thread, we can see that the policy based design of class aint facilitates to discover, that a NaNPolicy could be applicable not only to a single int implementation or a whole set of integer implementations but also to number implementations in general. Moreover we may discover, that a NaNPolicy is not the only addition or "completion", that people many desire for numbers. I'd like to call those classes "closures", because they try to close or complete the behavior of a data type in a defined way where is has been undefined before. [BTW I don't like those and I am not advocating to use them, but they are a nice vehicle for some design aspects that we can study here.] So I generalized this template parameter a little calling it 'class Closure' now instead of 'class NaNPolicy'. (I suggested this idea earlier http://lists.boost.org/Archives/boost/2010/04/164816.php but didn't make sure it was well received) On 04/07/2010 05:04 PM, Joachim Faulhaber wrote:
Maybe it would be more interesting to provide the minimal and "pure" numeric type and provide a NaN and Infinity closure as template
template<class IntegralT, class ClosureTraits=...> closed { ... }
because the way in which NaNs, Infinities (and maybe more special values) are handled should be the same for all the different implementations of integral numeric types.
Another aspect I wanted to develop further is (4) Reduction of code replication. In the first example of 'class aint' we have still code replication in of the form template<class Rep> inline static void o_assign(Rep& x, Rep const& y) { if (is_nan(x)) ; else if(is_nan(y)) x = y; else x o= y; // only 'o' varies } The "skeleton" 'if(is_nan(x)); else if(is_nan(y))...' replicates for all implementations of e.g. operators o= Within our design we get rid of it by lifting all operations of identical signatures like binary op_assign operators: We encapsulate them in functor classes like template<class Type> struct add_to { Type& operator()(Type& x, Type const& y) { return x += y; } }; and pass them via template parameters: template<class Rep, template<class>class Op> inline static void op_assign(Rep& x, Rep const& y) { if (is_nan(x)) ; else if(is_nan(y)) x = y; else Op<Rep>()(x, y); } So here is the enhanced example of 'class aint' using policy based design: //========================================================== struct nany; // Takes care for the nasty nan struct manly; // No, we don't want this template<class, class> class aint; // a int with closure policies //---------------------------------------------------------- // Functor classes for op_assign operators to reduce // code replication. template<class Type> struct add_to { Type& operator()(Type& x, Type const& y) { return x += y; } }; template<class Type> struct subtract_from { Type& operator()(Type& x, Type const& y) { return x -= y; } }; // ... also |=, *= etc. //---------------------------------------------------------- // An int class with Closure template<class Rep, class Closure = manly> class aint // no sunshine when she's gone { public: aint(): _value(){} aint(Rep const& val): _value(val){} aint& operator /= (aint const& y) { Closure::div_assign(_value, y._value); return *this; } aint& operator += (aint const& y) { Closure::op_assign<Rep,add_to>(_value, y._value); return *this; } aint& operator -= (aint const& y) { Closure::op_assign <Rep,subtract_from>(_value, y._value); return *this; } Rep value()const { return _value; } private: Rep _value; }; // Our Closure policy class can also be used in // free standing functions template<class CharType, class CharTraits, class Rep, class Closure> std::basic_ostream<CharType, CharTraits>& operator << (std::basic_ostream<CharType, CharTraits> &stream, aint<Rep,Closure> const& x) { return Closure::to_stream <CharType,CharTraits,Rep>(stream, x.value()); } struct nany { // Improvement: all the nan stuff now with the // nan-Closure class template<class Rep> inline static Rep nan() { // Again not perfect, ... ok for the example return ( numeric_limits<Rep>::has_quiet_NaN ? (numeric_limits<Rep>::quiet_NaN)() : (numeric_limits<Rep>::max)() ); } template<class Rep> inline static bool is_nan(const Rep& x) { return x == nan<Rep>(); } template<class Rep> inline static void div_assign(Rep& x, Rep const& y) { if (is_nan(x)) ; else if(is_nan(y)) x = y; else if(y == 0) x = nan<Rep>(); else x /= y; } // Code replication: For all op_assign operators Op // we have only one implementation template<class Rep, template<class>class Op> inline static void op_assign(Rep& x, Rep const& y) { if (is_nan(x)) ; else if(is_nan(y)) x = y; else Op<Rep>()(x, y); } template<class CharType, class CharTraits, class Rep> static std::basic_ostream<CharType, CharTraits>& to_stream(std::basic_ostream<CharType, CharTraits> &stream, Rep const& x) { if(is_nan(x)) return stream << "NaN"; else return stream << x; } }; struct manly { template<class Rep> inline static void div_assign(Rep& x, Rep const& y) { x /= y; } template<class Rep, template<class>class Op> inline static void op_assign(Rep& x, Rep const& y) { Op<Rep>()(x, y); } template<class CharType, class CharTraits, class Rep> static std::basic_ostream<CharType, CharTraits>& to_stream(std::basic_ostream<CharType, CharTraits> &stream, Rep const& x) { return stream << x; } }; BOOST_AUTO_TEST_CASE(nan_policy_study) { // No NaN protection aint<long> ago; // aint<short, nany> sun(2); aint<short, nany> shine(7); sun += shine; cout << "2+7 = " << sun << endl; shine /= 0; cout << "7/0 = " << shine << endl; } //========================================================== I hope this is not too, well, overwhelming and still helpful ;-/ Cheers, Joachim -- Interval Container Library [Boost.Icl] http://www.joachim-faulhaber.de
On Sat, 12 Mar 2011 22:36:58 +0100 Joachim Faulhaber <afojgo@googlemail.com> wrote:
With respect to the side note of Jan, I'd like to resume the example of a policy based design. As we are separating concerns and encapsulate them into policy classes, we should be able to code a behavior like the MS SafeInt mentioned by Jan in a policy class.
typedef aint<int, MsSafeIntBehavior> aSafeInt; typedef aint<xint::integer_t<options::fixedlenth<512> >, MsSafeIntBehavior> anXSafeInt; [...]
I haven't studied the original article yet, but if I'm reading that right, MsSafeIntBehavior is just wrapping the type specified by the initial parameter. That would neatly solve at least one of the problems I was pondering. I'll examine these carefully when I have a chance to do so.
Another aspect I wanted to develop further is
(4) Reduction of code replication.
In the first example of 'class aint' we have still code replication [...]
Ahh... I'd come up with something similar, if cruder, while thinking about your initial message. Nice to see I was on the right track.
I hope this is not too, well, overwhelming and still helpful ;-/
I think it will be, once I sit down and work with it a while. Thank you. -- Chad Nelson Oak Circle Software, Inc. * * *
participants (3)
-
Chad Nelson -
JCK -
Joachim Faulhaber