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