ENH: Removed duplication on GetPlaneFromBond.

Also added a little extra const safety to the atom and bond classes.

Change-Id: I46f529b0c185da65b92523dcd54ff6e26cf52538

Filtering/vtkAtom.cxx

@@ -42,7 +42,7 @@ void vtkAtom::PrintSelf(ostream &os, vtkIndent indent)
 }
 
 //----------------------------------------------------------------------------
-unsigned short vtkAtom::GetAtomicNumber()
+unsigned short vtkAtom::GetAtomicNumber() const
 {
   return this->Molecule->GetAtomAtomicNumber(this->Id);
 }
@@ -54,13 +54,13 @@ void vtkAtom::SetAtomicNumber(unsigned short atomicNum)
 }
 
 //----------------------------------------------------------------------------
-void vtkAtom::GetPosition(float pos[3])
+void vtkAtom::GetPosition(float pos[3]) const
 {
   this->Molecule->GetAtomPosition(this->Id, pos);
 }
 
 //----------------------------------------------------------------------------
-void vtkAtom::GetPosition(double pos[3])
+void vtkAtom::GetPosition(double pos[3]) const
 {
   vtkVector3f position = this->GetPosition();
   pos[0] = position.X();
@@ -81,7 +81,7 @@ void vtkAtom::SetPosition(float x, float y, float z)
 }
 
 //----------------------------------------------------------------------------
-vtkVector3f vtkAtom::GetPosition()
+vtkVector3f vtkAtom::GetPosition() const
 {
   return this->Molecule->GetAtomPosition(this->Id);
 }

Filtering/vtkAtom.h

@@ -33,30 +33,24 @@ class VTK_FILTERING_EXPORT vtkAtom
 
   // Description:
   // Return the Id used to identify this atom in the parent molecule.
-  vtkIdType GetId()
-  {
-    return this->Id;
-  }
+  vtkIdType GetId() const;
 
   // Description:
   // Return the parent molecule of this atom.
-  vtkMolecule * GetMolecule()
-  {
-    return this->Molecule;
-  }
+  vtkMolecule * GetMolecule();
 
   // Description:
   // Get/Set the atomic number of this atom
-  unsigned short GetAtomicNumber();
+  unsigned short GetAtomicNumber() const;
   void SetAtomicNumber(unsigned short atomicNum);
 
   // Description:
   // Get/Set the position of this atom
-  void GetPosition(float pos[3]);
-  void GetPosition(double pos[3]);
+  void GetPosition(float pos[3]) const;
+  void GetPosition(double pos[3]) const;
   void SetPosition(const float pos[3]);
   void SetPosition(float x, float y, float z);
-  vtkVector3f GetPosition();
+  vtkVector3f GetPosition() const;
   void SetPosition(const vtkVector3f &pos);
 
  protected:
@@ -68,4 +62,14 @@ class VTK_FILTERING_EXPORT vtkAtom
   vtkIdType Id;
 };
 
+inline vtkIdType vtkAtom::GetId() const
+{
+  return this->Id;
+}
+
+inline vtkMolecule * vtkAtom::GetMolecule()
+{
+  return this->Molecule;
+}
+
 #endif

Filtering/vtkBond.cxx

@@ -48,7 +48,7 @@ void vtkBond::PrintSelf(ostream &os, vtkIndent indent)
 }
 
 //----------------------------------------------------------------------------
-double vtkBond::GetLength()
+double vtkBond::GetLength() const
 {
   // Reimplement here to avoid the potential cost of building the EdgeList
   // (We already know the atomIds, no need to look them up)
@@ -58,6 +58,18 @@ double vtkBond::GetLength()
   return (pos2 - pos1).Norm();
 }
 
+//----------------------------------------------------------------------------
+vtkIdType vtkBond::GetBeginAtomId() const
+{
+  return this->BeginAtomId;
+}
+
+//----------------------------------------------------------------------------
+vtkIdType vtkBond::GetEndAtomId() const
+{
+  return this->EndAtomId;
+}
+
 //----------------------------------------------------------------------------
 vtkAtom vtkBond::GetBeginAtom()
 {
@@ -70,6 +82,18 @@ vtkAtom vtkBond::GetEndAtom()
   return this->Molecule->GetAtom(this->EndAtomId);
 }
 
+//----------------------------------------------------------------------------
+const vtkAtom vtkBond::GetBeginAtom() const
+{
+  return this->Molecule->GetAtom(this->BeginAtomId);
+}
+
+//----------------------------------------------------------------------------
+const vtkAtom vtkBond::GetEndAtom() const
+{
+  return this->Molecule->GetAtom(this->EndAtomId);
+}
+
 //----------------------------------------------------------------------------
 unsigned short vtkBond::GetOrder()
 {

Filtering/vtkBond.h

@@ -31,34 +31,24 @@ public:
 
   // Description:
   // Return the Id used to identify this bond in the parent molecule.
-  vtkIdType GetId()
-  {
-    return this->Id;
-  }
+  vtkIdType GetId() const;
 
   // Description:
   // Return the parent molecule of this bond.
-  vtkMolecule * GetMolecule()
-  {
-    return this->Molecule;
-  }
+  vtkMolecule * GetMolecule();
 
   // Description:
   // Get the starting / ending atom ids for this bond.
-  vtkIdType GetBeginAtomId()
-  {
-    return this->BeginAtomId;
-  }
-  vtkIdType GetEndAtomId()
-  {
-    return this->EndAtomId;
-  }
+  vtkIdType GetBeginAtomId() const;
+  vtkIdType GetEndAtomId() const;
 
   // Description:
   // Get a vtkAtom object that refers to the starting / ending atom
   // for this bond.
   vtkAtom GetBeginAtom();
   vtkAtom GetEndAtom();
+  const vtkAtom GetBeginAtom() const;
+  const vtkAtom GetEndAtom() const;
 
   // Description:
   // Get the bond order for this bond.
@@ -69,7 +59,7 @@ public:
   //
   // @note This function is faster than vtkMolecule::GetBondLength and
   // should be used when possible.
-  double GetLength();
+  double GetLength() const;
 
 protected:
   friend class vtkMolecule;
@@ -83,4 +73,14 @@ protected:
   vtkIdType EndAtomId;
 };
 
+inline vtkIdType vtkBond::GetId() const
+{
+  return this->Id;
+}
+
+inline vtkMolecule * vtkBond::GetMolecule()
+{
+  return this->Molecule;
+}
+
 #endif

Filtering/vtkMolecule.cxx

@@ -399,8 +399,18 @@ void vtkMolecule::UpdateBondList()
   this->BondListIsDirty = false;
 }
 
-bool vtkMolecule::GetPlaneFromBond(vtkAtom atom1, vtkAtom atom2,
-                                   const double normal[3], vtkPlane *plane)
+//----------------------------------------------------------------------------
+bool vtkMolecule::GetPlaneFromBond(const vtkBond &bond,
+                                   const vtkVector3f &normal,
+                                   vtkPlane *plane)
+{
+  return vtkMolecule::GetPlaneFromBond(bond.GetBeginAtom(), bond.GetEndAtom(),
+                                       normal, plane);
+}
+
+//----------------------------------------------------------------------------
+bool vtkMolecule::GetPlaneFromBond(const vtkAtom &atom1, const vtkAtom &atom2,
+                                   const vtkVector3f &normal, vtkPlane *plane)
 {
   if (plane == NULL)
     {
@@ -409,7 +419,7 @@ bool vtkMolecule::GetPlaneFromBond(vtkAtom atom1, vtkAtom atom2,
 
   vtkVector3f v(atom1.GetPosition() - atom2.GetPosition());
 
-  vtkVector3f n_i(vtkVector3d(normal).Cast<float>().GetData());
+  vtkVector3f n_i(normal);
   vtkVector3f unitV(v.Normalized());
 
   // Check if vectors are (nearly) parallel
@@ -427,10 +437,7 @@ bool vtkMolecule::GetPlaneFromBond(vtkAtom atom1, vtkAtom atom2,
   // end vtkVector reimplementation TODO
 
   // Calculate actual normal:
-  // TODO Remove/restore this when subtraction is supported again
-  // vtkVector3d realNormal (n_i - proj);
-  vtkVector3f realNormal (n_i[0]-proj[0], n_i[1]-proj[1], n_i[2]-proj[2]);
-  // end vtkVector reimplementation TODO
+  vtkVector3f realNormal(n_i - proj);
 
   // Create plane:
   vtkVector3f pos(atom1.GetPosition());

Filtering/vtkMolecule.h

@@ -81,7 +81,7 @@ class vtkUnsignedShortArray;
 
 class VTK_FILTERING_EXPORT vtkMolecule : public vtkUndirectedGraph
 {
- public:
+public:
   static vtkMolecule *New();
   vtkTypeMacro(vtkMolecule,vtkUndirectedGraph);
   void PrintSelf(ostream &os, vtkIndent indent);
@@ -212,8 +212,7 @@ class VTK_FILTERING_EXPORT vtkMolecule : public vtkUndirectedGraph
 
   // Description:
   // Obtain the plane that passes through the indicated bond with the given
-  // normal. If the plane is set successfully, the function return true.
-  //
+  // normal. If the plane is set successfully, the function returns true.
   //
   // If the normal is not orthogonal to the bond, a new normal will be
   // constructed in such a way that the plane will be orthogonal to
@@ -237,72 +236,10 @@ class VTK_FILTERING_EXPORT vtkMolecule : public vtkUndirectedGraph
   //
   // If n_i is parallel to v, a warning will be printed and no plane will be
   // added. Obviously, n_i must not be parallel to v.
-  static bool GetPlaneFromBond(vtkBond bond, const double normal[3],
-                               vtkPlane *plane)
-  {
-    return vtkMolecule::GetPlaneFromBond(bond.GetBeginAtom(),
-                                         bond.GetEndAtom(), normal, plane);
-  }
-  static bool GetPlaneFromBond(vtkBond bond, const float normal[3],
-                               vtkPlane *plane)
-  {
-    double normald[3];
-    normald[0] = static_cast<double>(normal[0]);
-    normald[1] = static_cast<double>(normal[1]);
-    normald[2] = static_cast<double>(normal[2]);
-    return vtkMolecule::GetPlaneFromBond(bond, normald, plane);
-  }
-  static bool GetPlaneFromBond(vtkBond bond,
-                               double n_x, double n_y, double n_z,
-                               vtkPlane *plane)
-  {
-    double normal[3];
-    normal[0] = n_x;
-    normal[1] = n_y;
-    normal[2] = n_z;
-    return vtkMolecule::GetPlaneFromBond(bond, normal, plane);
-  }
-  static bool GetPlaneFromBond(vtkBond bond, vtkVector3d normal,
-                               vtkPlane *plane)
-  {
-    return vtkMolecule::GetPlaneFromBond(bond, normal.GetData(), plane);
-  }
-  static bool GetPlaneFromBond(vtkBond bond, vtkVector3f normal,
-                               vtkPlane *plane)
-  {
-    return vtkMolecule::GetPlaneFromBond(bond, normal.GetData(), plane);
-  }
-  static bool GetPlaneFromBond(vtkAtom atom1, vtkAtom atom2,
-                             const double normal[3], vtkPlane *plane);
-  static bool GetPlaneFromBond(vtkAtom atom1, vtkAtom atom2,
-                             const float normal[3], vtkPlane *plane)
-  {
-    double normald[3];
-    normald[0] = static_cast<double>(normal[0]);
-    normald[1] = static_cast<double>(normal[1]);
-    normald[2] = static_cast<double>(normal[2]);
-    return vtkMolecule::GetPlaneFromBond(atom1, atom2, normald, plane);
-  }
-  static bool GetPlaneFromBond(vtkAtom atom1, vtkAtom atom2,
-                               double n_x, double n_y, double n_z,
-                               vtkPlane *plane)
-  {
-    double normal[3];
-    normal[0] = n_x;
-    normal[1] = n_y;
-    normal[2] = n_z;
-    return vtkMolecule::GetPlaneFromBond(atom1, atom2, normal, plane);
-  }
-  static bool GetPlaneFromBond(vtkAtom atom1, vtkAtom atom2,
-                             vtkVector3d normal, vtkPlane *plane)
-  {
-    return vtkMolecule::GetPlaneFromBond(atom1,atom2,normal.GetData(),plane);
-  }
-  static bool GetPlaneFromBond(vtkAtom atom1, vtkAtom atom2,
-                             vtkVector3f normal, vtkPlane *plane)
-  {
-    return vtkMolecule::GetPlaneFromBond(atom1,atom2,normal.GetData(),plane);
-  }
+  static bool GetPlaneFromBond(const vtkBond &bond, const vtkVector3f &normal,
+                               vtkPlane *plane);
+  static bool GetPlaneFromBond(const vtkAtom &atom1, const vtkAtom &atom2,
+                               const vtkVector3f &normal, vtkPlane *plane);
 
  protected:
   vtkMolecule();
@@ -330,7 +267,7 @@ class VTK_FILTERING_EXPORT vtkMolecule : public vtkUndirectedGraph
 
   vtkAbstractElectronicData *ElectronicData;
 
- private:
+private:
   vtkMolecule(const vtkMolecule&);    // Not implemented.
   void operator=(const vtkMolecule&); // Not implemented.
 };