BIND 10 #2432: define and implement base and libdns++ version of RRsetCollection

BIND 10 Development do-not-reply at isc.org
Mon Dec 31 05:35:23 UTC 2012


#2432: define and implement base and libdns++ version of RRsetCollection
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  vorner
            Priority:  medium        |                       Status:
           Component:  libdns++      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130108
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  5             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  loadzone-ng
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by muks):

 * owner:  muks => vorner


Comment:

 Hi Michal

 Replying to [comment:11 vorner]:
 > Whatever it is, it hardly can be copied like this. So the current
 interface
 > seems to limiting. I don't know if it's reasonable to merge it in this
 way if
 > we know it would have to be changed in the next ticket anyway. One thing
 is
 > when we don't know about a problem with interface and it is then changed
 as a
 > result of discovering the problem. But now, we know about the problem,
 so we
 > shouldn't have code we know is broken.

 I agree it can't be copied. However, I don't know how the datasrc
 implementation
 is going to use this. We can remove the `std::iterator` interface and get
 rid of
 suffix `operator++`, and also make it non-copyable. Is this what you
 suggest?

 I'd like to fix this properly instead of just doing something to move
 along,
 but for that I need to know how this interface is going to be used. This
 is why
 I think it's best to do it once as part of the datasrc implementation. But
 if you
 think the above is enough, I'll implement that.

 > > `RRClass` has been added to all `find()` variants now (and we use it
 in the collection key).
 >
 > Yes. Regarding that commit, the different orders of arguments quite
 scare me.
 > For one, it is really annoying to remember why something has one order
 and
 > something else a different one. But the bigger problem is, if they
 return
 > different things, it is very easy to call the wrong one by accident.
 >
 > Is there any reason why we need two different versions anyway?

 I guess you understand already what the two methods return, and the
 `const` interface
 in the base class vs. the `RRsetPtr` interface in the derived class.

 Even in this case I don't know what the intent is and
 [[ZoneLoadingAPIDesign]] is not
 clear about why the libdns++ `RRsetCollection` exists or where its methods
 are supposed
 to be used.

 > > I may not follow what you mean by 'different types of collections'
 here.
 > > I am assuming it is different implementations of
 `RRsetCollectionBase`. I guess we
 > > won't be comparing two different iterators in our code like that, but
 regardless,
 > > the code was incorrect as it could throw. `equals()` has been modified
 to return
 > > inequality in this case without throwing, and I've also added a test
 with a mock
 > > `RRsetCollectionBase` implementation for it.
 >
 > Yes, I meant that. From the interface point of view, it is now fixed.
 But I
 > don't really like the way how it is fixed. This requires each
 implementator of
 > each derived class to check it and not forget. Could that be checked on
 the
 > base class level somehow?

 As these iterator implementations are tied to the `RRsetCollectionBase`
 implementations,
 I'm not sure how you can check for the valid implementation type in the
 base class.

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


More information about the bind10-tickets mailing list