BIND 10 #2376: define LoaderContext class

[BIND 10 Development]

#2376: define LoaderContext class
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20121120
  medium                             |            Resolution:
                  Component:  data   |             Sensitive:  0
  source                             |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  4
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
  scalable inmemory                  |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:19 vorner]:

 > I understand it in the case of the context, which is somehow heavyweight
 and
 > primarily oriented towards adding RRsets. But with this lightweight
 almost
 > struct, I don't see the real problem. If the Rdata constructors act the
 same

 If we don't see a problem in that sense with the revised
 `LoaderCallbacks`,
 I wouldn't see it with `MasterLoaderContextBase` either.  In fact, the
 complexity of the context class is irrelevant to such users as rdata
 constructors; in either case it would just call corresponding public
 interface to get access to the necessary callbacks, however
 complicated its internal is.

 > way as html renderers ‒ if you don't understand it, ignore it ‒ it
 should be no
 > problem. It might seem slightly odd, but if we split it, it will become
 less
 > convenient to use.

 My biggest concern is that rdata implementations would depend on
 a higher level concept of `RRset`, and they cannot just consider it an
 opaque type they don't use because for the definition of `RRsetPtr`
 they need to include rrset.h (indirectly).

 > If you still think it is worth splitting it off, there could be two
 options:
 >  * Passing two separate parameters to the MasterLoader constructor (the
 less convenient way).

 Personally I don't see a problem with this - as I said above, the
 tradeoff for the split approach is independent from the complexity of
 the context or the call back class/struct to me.  But that's probably
 a matter of preference.

 >  * Inheritance. The class containing the issues is the base class and
 then
 >    there's a derived class adding the add callback. The master loader
 gets the
 >    derived one, which gives it all the needed callbacks. Then the
 constructors
 >    take only the base one, getting only the issue ones.

 This doesn't seem to be a good use of inheritance IMO.

 Another approach is to introduce a "forward" header that only includes
 forward declarations of basic types of lib(b10-)dns++ and typedefs of
 some (smart) pointer types.  We can then simply include this header
 from loader_callbacks.h because it doesn't need the definition of
 `RRset`.  In that case I think I can live with the compound single
 structure.  And, in any case, I think it's better to have something
 like this kind of "forward" header to help reduce dependency, even if
 not done in this ticket.

 BTW, I just noticed one another thing: I'm afraid the name
 "loader_callbacks.h" and `LoaderCallbacks` are too generic.  I'd
 rename them, e.g., "master_loader_callbacks.h" and
 `MasterLoaderCallbacks`, respectively.

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