BIND 10 #977: ACL base class

[BIND 10 Development]

#977: ACL base class
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  vorner                             |                Status:  reviewing
                       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      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Replying to [comment:5 jinmei]:
 >   - whether this style of mixture of inheritance and template is the
 >     best or quite better abstraction of what we'd like to achieve

 Well, having this kind of hierarchy for single context (if we want
 different kinds of checks) looks like textbook example for polymorphism.

 And while I believe we want to share code with different contexts where it
 makes sense, parts of the hierarchy will be completely different. And it
 would be problematic to write some generic base class for context, check
 every time it is the correct child, typecast it… So this looks like a
 textbook example for templates. So I just put them together into one
 class.

 >   - 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.

 Why so? And is 3 so deep? Most GUI components in whatever library has
 depths around 15.

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

 What is the rationale behind this? There's no constructor, so nothing to
 make hidden really. And the compiler won't let to create this class
 directly anyway, as it has pure virtual methods, so the goal to stop
 people from creating it by accident is fulfilled automatically as well. Is
 there any other reason I don't see?

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

 Like this?

 > - 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.

 This was an error, thanks for noticing. I wrote the header and comment
 only, but forgot even the implementation.

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