In my last commit regarding boost::shared_ptr, I left TextClassPtr as a
silly typedef:
typedef TextClass * TextClassPtr;
The attached would turn it into something akin to a "strong typedef",
where TextClassPtr really just wraps a TextClass*. The point of this is
to enforce the distinction between TextClasses generally, which can
represent various things (see the comment in TextClass.h), and
TextClasses that go with Buffers. A TextClassPtr is thus what is held in
the TextClassBundle, which just holds pointers to the TextClasses that
are associated with Buffers. This (a) keeps them alive even when the
Buffer itself is deleted but (b) keeps them from being leaked (though
we're never, at the moment, deleting them---that's the cost of
abandoning boost::shared_ptr---though maybe we could).
If this seems like over-engineering, well, I'm sure it is, a bit. On the
other hand, looking back through this code again has made me think the
differences between the various things TextClasses represent need to be
made more apparent, and this seems like a good way to do it. In the code
as it now stands, all access to the TextClasses that go with Buffers is
via TextClassPtr, but this doesn't enforce anything. With this new
class, we do require that all such TextClasses be represented by
TextClassPtrs, and we restrict creation of such TextClasses to
TextClassBundle itself. So the compiler will help us keep from confusing
things.
Comments welcome.
Richard
Index: src/insets/InsetCollapsable.cpp
===================================================================
--- src/insets/InsetCollapsable.cpp (revision 23232)
+++ src/insets/InsetCollapsable.cpp (working copy)
@@ -132,7 +132,7 @@
void InsetCollapsable::setLayout(TextClassPtr tc)
{
textClass_ = tc;
- if ( textClass_ != 0 ) {
+ if (textClass_.isNonNull()) {
layout_ = &textClass_->insetLayout(name());
labelstring_ = layout_->labelstring();
} else {
Index: src/insets/InsetCollapsable.h
===================================================================
--- src/insets/InsetCollapsable.h (revision 23232)
+++ src/insets/InsetCollapsable.h (working copy)
@@ -42,7 +42,7 @@
InsetCollapsable(
BufferParams const &,
CollapseStatus status = Inset::Open,
- TextClassPtr tc = 0
+ TextClassPtr tc = TextClassPtr()
);
///
InsetCollapsable(InsetCollapsable const & rhs);
Index: src/TextClass.cpp
===================================================================
--- src/TextClass.cpp (revision 23232)
+++ src/TextClass.cpp (working copy)
@@ -1133,9 +1133,8 @@
TextClassPtr TextClassBundle::newClass(TextClass const & baseClass)
{
- TextClass * tc = new TextClass(baseClass);
- tc_list_.push_back(tc);
- return tc;
+ tc_list_.push_back(TextClassPtr(baseClass));
+ return tc_list_.back();
}
@@ -1151,10 +1150,26 @@
std::list<TextClassPtr>::iterator it = tc_list_.begin();
std::list<TextClassPtr>::iterator end = tc_list_.end();
for (; it != end; ++it)
- delete *it;
+ it->release();
}
+TextClassPtr::TextClassPtr() :
+ ptr_(static_cast<TextClass *>(0))
+{}
+
+
+TextClassPtr::TextClassPtr(TextClass const & tc)
+{
+ ptr_ = new TextClass(tc);
+}
+
+
+void TextClassPtr::release() {
+ delete ptr_;
+ ptr_ = static_cast<TextClass *>(0);
+}
+
ostream & operator<<(ostream & os, PageSides p)
{
switch (p) {
Index: src/TextClassPtr.h
===================================================================
--- src/TextClassPtr.h (revision 23232)
+++ src/TextClassPtr.h (working copy)
@@ -4,6 +4,8 @@
* This file is part of LyX, the document processor.
* Licence details can be found in the file COPYING.
*
+ * \author Richard Heck
+ *
* Full author contact details are available in file CREDITS.
*/
#ifndef TEXTCLASS_PTR_H
@@ -13,9 +15,47 @@
class TextClass;
-// This largely useless typedef is scheduled to be replaced by
-// something better.
-typedef TextClass * TextClassPtr;
+/// This class amounts to little more than a `strong typedef'.
+/// Its purpose is to control the creation of TextClass objects
+/// within the TextClassBundle.
+/// These TextClasses represent the layout information that is
+/// associated with a given buffer.
+class TextClassPtr {
+// Implementations are in TextClass.cpp
+public:
+ /// This is for use ONLY by constructors of classes that have
+ /// TextClassPtr members or for providing default arguments.
+ /// It's otherwise useless to call it, since there is no way
+ /// to modify the null TextClassPtr that it creates.
+ TextClassPtr();
+ ///
+ TextClass & operator*() { return *ptr_; }
+ ///
+ TextClass const & operator*() const { return *ptr_; }
+ ///
+ TextClass * operator->() { return ptr_; }
+ ///
+ TextClass const * operator->() const { return ptr_; }
+ ///
+ bool operator==(TextClassPtr const & rhs) const
+ { return ptr_ == rhs.ptr_; }
+ ///
+ bool operator!=(TextClassPtr const & rhs) const
+ { return !(*this == rhs); }
+ /// \return True, if embedded pointer is non-null and it is
+ /// safe to dereference, etc.
+ bool isNonNull() const { return ptr_ != 0; }
+private:
+ /// Constructs a new TextClass from an old one.
+ TextClassPtr(TextClass const & tc);
+ /// Deletes the embedded pointer.
+ void release();
+ ///
+ TextClass * ptr_;
+ /// The only class that can really create a TextClassPtr is
+ /// TextClassBundle, which calls the private constructor.
+ friend class TextClassBundle;
+};
} // namespace lyx
