BIND 10 #977: ACL base class

BIND 10 Development do-not-reply at isc.org
Thu Jun 9 08:48:19 UTC 2011


#977: ACL base class
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  vorner                             |                Status:  accepted
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20110614
                  Component:         |            Resolution:
  Unclassified                       |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  0.0
Feature Depending on Ticket:  ACL    |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 '''General'''

 I was not sure if this design is good or not just from this set of
 definitions, including
   - whether this style of mixture of inheritance and template is the
     best or quite better abstraction of what we'd like to achieve
   - the fact that having both Check and CompoundCheck are a base class
     indicates we'll have quite deep levels of inheritance hierarchy.
     without a care such a design can be difficult to manage.

 But, not to waste time, I'd like to be clear that these are not
 objections.  After all, we won't be able to be sure about these points
 until we implement more specific cases.  So, I'm okay with moving
 forward with it for now.   You may or may not want to answer these
 points at this stage, but I don't request resolving the discussion as
 a prerequisite for merge.

 '''Specific comments'''

 - I made some trivial editorial changes and pushed them directly.
   Please check.

 - To describe template parameters for Check and CompoundCheck, I'd use
   typename (instead of class) if types that are not a "pure" class can
   be used as "Context" as is the case in the test.  This is basically
   a matter of taste, however, and it's up to you.

 - maybe it's better to make the constructor of the base class protected.

 - s/subexpressions()/getSubexpressions()/ according to our naming
   guideline.

 - I'd like to have descriptions about possible exceptions, at least
   for some important methods such as matches().

 - is it intentional that Check::toText() is *not* virtual?  If it's
 intentional,
   the fact that this class has a pure virtual method will cause an
   awkward effect, i.e., we'll have to provide the implementation of
   the pure virtual method, although it's not prohibited.

 - I'd avoid using hardcode for 10000.  I'd instead define a class
   static constant and refer to it anywhere else.

-- 
Ticket URL: <http://bind10.isc.org/ticket/977#comment:5>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development


More information about the bind10-tickets mailing list