On 07/10/2014 02:48 AM, Neil wrote:
> Except for refcounted base classes (which as you note need to have a
> protected virtual destructor), is there a correct style as to whether
> the destructor should be private or protected and virtual or nonvirtual?

IMO, the destructor should be nonvirtual, since we're only expecting to
be destroyed via the concrete class's ::Release() method.  So we
shouldn't need a virtual destructor.  (I'm not sure how best to *ensure*
that we're never destroyed via "delete pointerOfSuperclassType"
though... We could give all of the superclasses protected destructors,
which would help, but not be 100% foolproof.)

As for protected vs. private --  in the concrete class, I don't think it
particularly matters, since the keywords have the same effect, as long
as the class is MOZ_FINAL.

IIRC, when I was actively working on this cleanup, if we already had an
already-existing "protected" section like below...

class Foo MOZ_FINAL : public Bar {
 public:
   [...]
 protected:
   // overrides of protected methods from superclass
   // (in section labeled as "protected" for consistency w/ superclass)
   [...]
};

...then I just used the existing "protected" area, for conciseness,
rather than adding a new "private" area.  But if there wasn't any
consistency-with-my-superclass argument to be made for using
"protected", then I'd just use private, to avoid the misleading
implication that we have subclasses which can see *our* protected stuff.

>> First, if your class is abstract, then it shouldn't have
>> AddRef/Release implementations to begin with.  Those belong on the
>> concrete subclasses -- not on your abstract base class.
>>
> What's correct code for abstract class Foo (implementing interfaces
> IFoo1 and IFoo2) with derived classes Bar and Baz (implementing
> interfaces IBar1 and IBar2 or IBaz1 and IBaz2 respectively)?

Shouldn't the refcounting still be on the concrete classes? Or does that
not work for some reason?  Based on your question, I'm guessing it
doesn't. Anyway, I'm not sure what's supposed to happen there. :)

~Daniel
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Reply via email to