[IMP-dev] warning on unused restraints

[Daniel Russel <>]

Since I got bitten by forgetting to add a restraint to a model I added a 
warning. Restraint will print out a message if it is destroyed without 
ever having been added to a Model. This warning can be disabled for a 
restraint by calling Restraint::set_was_owned(true).
     
Index: kernel/include/IMP/Restraint.h
===================================================================
--- kernel/include/IMP/Restraint.h	(revision 672)
+++ kernel/include/IMP/Restraint.h	(working copy)
@@ -43,7 +43,7 @@
     constructor and should provide methods so that the set of particles
     can be modified after construction.
 
-    A restraint can be added to the model multiple times or to multiple 
+    A restraint can be added to the model multiple times or to multiple
     restraint sets in the same model.
 
     \note When logging is VERBOSE, restraints should print enough information
@@ -53,6 +53,13 @@
 
     \note Physical restraints should use the units of kcal/mol for restraint
     values and kcal/mol/A for derivatives.
+
+    \note Restraints will print a warning message if they are destroyed
+    without ever having been added to a model as this is an easy mistake
+    to make. To disable this warning for a particular restraint, call
+    set_was_owned(true).
+
+    \ingroup kernel
  */
 class IMPDLLEXPORT Restraint : public RefCountedObject
 {
@@ -96,9 +103,16 @@
   Model *get_model() const {
     IMP_assert(model_,
                "get_model() called before set_model()");
-    return model_.get();
+    return model_;
   }
 
+  /** A warning is printed if a restraint is destroyed
+      without ever having belonged to a restraint set or a model.
+   */
+  void set_was_owned(bool tf) {
+    was_owned_=tf;
+  }
+
   //! Return a list of sets of particles that are restrained by this restraint
   /** This function returns a list of sets of particles that are
       interacting within this restraint. Particles can appear in more
@@ -114,12 +128,20 @@
   IMP_LIST(protected, Particle, particle, Particle*)
 
 private:
-  Pointer<Model> model_;
+  /* This pointer should never be ref counted as Model has a
+     pointer to this object. Not that Model is refcounted yet.
+   */
+  Model *model_;
 
   /* True if restraint has not been deactivated.
      If it is not active, evaluate should not be called
    */
   bool is_active_;
+
+  /* keep track of whether the restraint ever was in a model.
+     Give warnings on destruction if it was not.
+   */
+  bool was_owned_;
 };
 
 IMP_OUTPUT_OPERATOR(Restraint);
Index: kernel/src/Restraint.cpp
===================================================================
--- kernel/src/Restraint.cpp	(revision 672)
+++ kernel/src/Restraint.cpp	(working copy)
@@ -17,6 +17,7 @@
 
 Restraint::Restraint()
 {
+  model_=NULL;
   is_active_ = true; // active by default
 }
 
@@ -24,6 +25,12 @@
 //! Destructor
 Restraint::~Restraint()
 {
+  if (!was_owned_) {
+    // can't use virtual functions in the destructor
+    std::cerr << "Restraint " << this << " is being destroyed "
+              << "without ever having been added to a model."
+              << std::endl;
+  }
 }
 
 
@@ -43,9 +50,10 @@
 {
   IMP_assert(model==NULL || get_number_of_particles()==0
              || model == get_particle(0)->get_model()
-             || (model_ && model_.get() == model),
+             || (model_ && model_ == model),
              "Model* different from Particle Model*");
   model_=model;
+  was_owned_=true;
 }
 
 void Restraint::show(std::ostream& out) const