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      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:7 jinmei]:
 > - just give up making `getCallbacks()` const.  But in that case, I'd
 >   like to make the `Callbacks` structure safer (I discussed the issue
 >   in my own comments below).  Introducing a specific constructor,
 >   hide error/warn variables as private, and (e.g.) have its user call
 >   the callback via a wrapper method:

 Done

 > >  * This is another glue class that does nothing :-(. Not that I'd know
 what to
 > >    do with it at this stage, but it makes the code gradually
 unreadable.
 >
 > I didn't see any readability difficulty in the branch as someone who
 > basically knows what we will do with this code in later stages.  I
 > think it's a common case of the tradeoff about breaking a specific
 > feature into several development tasks, and of the discussion about
 > the reasonable "bitable" size of tasks.  I personally don't think this
 > one is too small, but as we discussed in the call the other day, in
 > future tasks we can discuss it before starting the sprint if someone
 > feels a particular task is too small for the overhead.

 I didn't mean the size of task now. It was clear from the ticket.

 But if someone comes to bind10 code, never seen it before, and tries to
 read
 through it (or debug it or something), every layer that does nothing, only
 joins things together, makes the reading harder. It seems generally better
 if
 these wrapper/glue classes are not needed and we could call things
 directly.

 > - I'm not sure if `source_byte` is of any use for error reporting
 >   purposes.  hmm, maybe to help identify the specific keyword in the
 >   `source_line` that causes the error?

 I'm not sure about it either, but it was in the proposed signature. Should
 I remove it?

 > - `addRRset`: I wonder we might now want to make the parameter
 >   `ConstRRsetPtr`.  Excluding the old version of in-memory
 >   implementation, our current implementations shouldn't need to modify
 >   the given rrset.  On the other hand, since the master loader
 >   shouldn't need to be able to use the rrset after passing it to
 >   `addRRset`, it might be more convenient for the user of the loader
 >   if it can be mutable.  If we keep it mutable, I'd note that
 >   `MasterLoader` releases the ownership of the rrset on calling
 >   `addRRset` and won't modify it.  I'd also note that `MasterLoader`
 >   never gives a NULL pointer.  I'd further note `rrset` wouldn't be
 >   accompanied with RRSIG (RRSIG `RRsets` will be passed separately in
 >   a separate call to `addRRset`).

 I think it makes sense to pass the mutable version, after all, we don't
 know
 what uses the loader might have in future and there's no technical nor
 logic
 reason to make it const.

 > - `addRrset` test: shouldn't we refill in expected_rrsets_ before the
 >   second round of test?

 I don't think I understand what you mean.

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