On Sun, Mar 30, 2008 at 05:41:23AM -0400, rgheck wrote:
> Andre Poenitz wrote:
>> On Sun, Mar 30, 2008 at 03:44:28AM -0400, rgheck wrote:
>>
>>> Most of what I read suggests that this is a no-no, on the ground that
>>> standard containers don't have virtual constructors. What's our view?
>>>
>>
>> One needs a virtual _de_structor exactly in the case when one is
>> deleting an object of the derived type using a pointer to the base type.
>> So if you don't do that (and make sure nobody else will, that's the hard
>> part ;-|) nothing evil will happen.
>> My rule of thumb is: "First try a data member and forwarding functions if
>> there are a few functions needed. If you are likely to end up with 50
>> trivial functions, private inheritance might be worth to consider. Stay
>> away from public inheritance unless you really know what you do."
>>
>>
> OK. In that case, what do you think of the attached? We've got a dozen
> forwarding functions. Whether anyone will ever do The Evil Thing, maybe
> not, but still.
>
> Actually, we may have been doing The Evil Thing. Some of my other reading
> suggested that this was dangerous:
> class Derived : public std::vector<Base>;
> Derived d = ...
> Derived::iterator it = ...;
> d.erase(it);
That should be save. As soon as you handle a 'Derived' always as
'Derived', and not as 'Base', all is fine.
> Is this the sort of thing you meant? or is it deleting Derived that's the
> issue?
> Index: EmbeddedFiles.h
> ===================================================================
> --- EmbeddedFiles.h (revision 24055)
> +++ EmbeddedFiles.h (working copy)
> @@ -197,8 +197,35 @@
> bool operator!=(EmbeddedFile const & lhs, EmbeddedFile const & rhs);
>
>
> -class EmbeddedFileList : public std::vector<EmbeddedFile> {
> +class EmbeddedFileList {
> public:
> + ///
> + typedef std::vector<EmbeddedFile>::iterator iterator;
> + ///
> + typedef std::vector<EmbeddedFile>::const_iterator const_iterator;
> + ///
> + iterator begin() { return eflist_.begin(); }
> + iterator end() { return eflist_.end(); }
> + const_iterator begin() const { return eflist_.begin(); }
> + const_iterator end() const { return eflist_.end(); }
> + ///
> + void push_back(EmbeddedFile const & ef) { eflist_.push_back(ef); }
> + ///
> + EmbeddedFile const & back() const { return eflist_.back(); }
> + EmbeddedFile & back() { return eflist_.back(); }
> + ///
> + void clear() { eflist_.clear(); }
> + ///
> + bool empty() const { return eflist_.empty(); }
> + ///
> + void insert(iterator position, const_iterator itbeg, const_iterator
> itend)
> + { eflist_.insert(position, itbeg, itend); }
> + void insert(iterator position, EmbeddedFile const & ef)
> + { eflist_.insert(position, ef); }
> + ///
> + iterator erase(iterator position) { return eflist_.erase(position); }
> +
> +
> /// set buffer params embedded flag. Files will be updated or extracted
> /// if such an operation fails, enable will fail.
> void enable(bool enabled, Buffer & buffer, bool updateFile);
> @@ -209,8 +236,8 @@
> */
> void registerFile(EmbeddedFile const & file, Inset const * inset,
> Buffer const & buffer);
> - /// returns a pointer to the Embedded file representing this object,
> - /// or null if not found. The filename should be absolute.
> + /// returns an iterator pointing to the Embedded file representing
> + /// the file with the absolute filename <filename>.
> const_iterator findFile(std::string const & filename) const;
> iterator findFile(std::string const & filename);
>
> @@ -220,6 +247,9 @@
> void update(Buffer const & buffer);
> /// write a zip file
> bool writeFile(support::DocFileName const & filename, Buffer const &
> buffer);
> +private:
> + ///
> + std::vector<EmbeddedFile> eflist_;
> };
First of all, you could use _private_ inheritance here. It would still
expose the fact that you are using an vector as underlying container,
and this fact would be end up in any .o file of any 'user code', but
it would make it less likely to 'accidentally' use the base class.
Secondly, 10 forwarded functions is not too much, so containment is ok.
Andre'
PS: The 'exposition' mentioned is not really a problem for LyX, as we
always compile everything and do not link-in precompiled third party
object files, so we do not need any kind of binary compatibility. This
is customarily needed in commercial and/or larger projects, though, and
I do not really see a point of not applying known-to-work-and-scale-well
concepts to LyX, too.
