BIND 10 #1575: move data_source.cc:Nsec3Param to libdns++

[BIND 10 Development]

#1575: move data_source.cc:Nsec3Param to libdns++
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120207
  critical                           |            Resolution:
                  Component:         |             Sensitive:  0
  libdns++                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  4
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:  NSEC3  |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 > > Does Doxygen generate the „brief“ descriptions from the first
 paragraph automatically? It seems so, but I'd prefer having them there,
 just for the clarity.
 >
 > Yes, it does, but I have no objection to explicitly adding it for
 > clarity.  Done.  (If that's our consensus we should probably document
 > it as part of guideline).

 +1 on that. Should we bring it to the call to announce/discuss?

 > Hmm, I'm okay with revising the message, but in that case I'd rather
 > be more consistent with messages for this type of error from other
 > Python built-in libraries.  I've changed them that way.

 Yes, true.

 > Do you mean introducing a base class and defining a factory function
 > (or method) like this?
 >
 > {{{#!c++
 > class NSEC3HashBase {
 > public:
 >     static NSEC3HashBase* create(const rdata::generic::NSEC3PARAM&
 param);
 >     virtual std::string calculate(const Name& name) const = 0;
 > };
 > }}}
 > (maybe we could name the base class `NSEC3Hash` and hide the concrete
 > one in the implementation).

 Yes, something like this, and I'd prefer hiding the concrete one.

 > But I'm not sure if we want to define derived classes for different
 > algorithms.  Again, as noted, my not-so-fixed intention is to allow
 > the application to switch parameters (including the algorithm) while
 > keeping the hash object itself so it can reuse the internal resource
 > as long as possible.  That would be possibly useful for validator,
 > which would create and hold a single `NSEC3Hash` object and switch the
 > parameter for each validation attempt.

 Wouldn't most of the resources be algorithm-dependant anyway?

 But what I propose is not forcing to have a separate one for each
 algorithm. I propose to have the static factory function to allow future
 flexibility, allowing us to choose then if we want to have a separate one
 for each or one for all of them, or something in between.

 > > Also, every time I see the python wrapper file generated form the
 template, I wonder if we should leave the comments about what should be
 done to fill in the template in, as this looks as something was forgotten
 to be done. But that might be for a wider discussion than just this
 ticket.
 >
 > Are you specifically talking about this one (for this branch)?

 I think it was this one, I can't find any other now.

 > This should have been removed, and I did it.  I tried to remove this
 > type of comments in the branch, but this one was overlooked.  Is there
 > anything else you noted?
 >
 > As a general issue, it may help if we mark such comments in the
 > template like this:
 > {{{#!c++
 > // THIS BLOCK OF COMMENT MUST BE REMOVED FROM THE PUBLISHED VERSION
 > // This is a template of typical code logic of python class
 initialization
 > // with C++ backend.  You'll need to adjust it according to details of
 the
 > // actual C++ class.
 > }}}

 Hopefully it'll help. Maybe it could even be not a comment, so the
 compiler could fail in case someone forgets to remove it?

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