Improve overall interface

src/CMI.cpp

@@ -1,149 +1,183 @@
 #include "CMI.hpp"
 
-namespace CMI{
+namespace CMI {
 
-    HalfMove::HalfMove(): number(1), isBlack(false){
+HalfMove::HalfMove()
+    : number(1), isBlack(false), NAG(0), parent(nullptr), mainline(nullptr) {}
+
+HalfMove::HalfMove(const std::string &SAN)
+    : number(1), isBlack(false), parent(nullptr), mainline(nullptr) {}
+
+HalfMove::HalfMove(const std::string &SAN, const std::string &comment,
+                   std::uint16_t number, std::uint8_t NAG, bool isBlack)
+    : SAN(SAN), comment(comment), number(number), NAG(NAG), isBlack(isBlack),
+      parent(nullptr), mainline(nullptr) {}
+
+HalfMove::~HalfMove() {
+  if (mainline != nullptr)
+    delete mainline;
+  for (HalfMove *v : variations)
+    delete v;
+}
+
+void HalfMove::SetParent(HalfMove *m) { parent = m; }
+
+std::vector<HalfMove *> HalfMove::GetVariations() const { return variations; }
+
+HalfMove *HalfMove::GetMainline() const { return mainline; };
+
+HalfMove *HalfMove::GetParent() const { return parent; };
+
+std::string HalfMove::GetSAN() const { return SAN; };
+
+void HalfMove::SetSAN(const std::string &newSAN) { SAN = newSAN; };
+
+std::uint16_t HalfMove::GetNumber() const { return number; };
+
+void HalfMove::SetNumber(std::uint16_t n) { number = n; };
+
+std::uint8_t HalfMove::GetNAG() const { return NAG; };
+
+void HalfMove::SetNAG(std::uint8_t n) { NAG = n; };
+
+std::string HalfMove::GetComment() const { return comment; };
+
+void HalfMove::SetComment(const std::string &c) { comment = c; };
+
+bool HalfMove::IsBlack() const { return isBlack; };
+
+void HalfMove::SetIsBlack(bool b) { isBlack = b; };
+
+void HalfMove::ClearVariations() { variations.clear(); }
+
+void HalfMove::SetMainline(HalfMove *m) {
+  mainline = m;
+  if (m != nullptr) {
+    if (!this->isBlack) {
+      m->SetIsBlack(true);
+      m->SetNumber(this->number);
+    } else {
+      m->SetIsBlack(false);
+      m->SetNumber(this->number + 1);
     }
+    m->SetParent(this);
+  }
+}
 
-    HalfMove::~HalfMove() {
+void HalfMove::Promote() {
-    if(mainline!=nullptr)
+  HalfMove *broot = GetBranchRoot();
-        delete mainline;
+  if (broot != nullptr) {
-    for(HalfMove *v:variations)
+    HalfMove *parent = broot->GetParent();
-        delete v;
+    if (parent != nullptr) {
-    }
+      if (parent->GetMainline() != broot) {
-
+        HalfMove *pparent = parent->GetParent();
-    void HalfMove::SetParent(CMI::HalfMove* m){
+        // First update parent of parent:
-        parent=static_cast<HalfMove*>(m);
+        if (pparent != nullptr) {
-    }
+          if (pparent->GetMainline() == parent)
-
+            pparent->SetMainline(broot);
-
+          else {
-    std::vector<CMI::HalfMove*> HalfMove::GetVariations() const {
+            pparent->AddVariation(broot);
-        std::vector<CMI::HalfMove*> vars;
+            pparent->RemoveChild(parent);
-        for(HalfMove *v:variations){
+          }
-            vars.push_back(static_cast<CMI::HalfMove*>(v));
         }
-        return vars;
+        // Now update parent:
+        parent->RemoveChild(broot);
+        broot->AddVariation(parent);
+      }
     }
-    void HalfMove::SetVariations(std::vector<CMI::HalfMove*> vars){
+  }
-        variations.clear();
+}
-        for(auto *v: vars){
+
-            variations.push_back(static_cast<HalfMove*>(v));
+void HalfMove::SetAsMainline() {
-        }
+  HalfMove *broot = GetBranchRoot();
+  HalfMove *lastRoot;
+  // Just promote until we cannot anymore
+  do {
+    lastRoot = broot;
+    broot->Promote();
+    broot = GetBranchRoot();
+  } while (broot != lastRoot);
+}
+
+HalfMove *HalfMove::GetBranchRoot() {
+  HalfMove *m = this;
+  HalfMove *p = parent;
+  while (p != nullptr) {
+    if (p->GetMainline() != m) {
+      return (m);
     }
+    m = p;
+    p = m->GetParent();
+  }
+  return m;
+}
 
+void HalfMove::AddVariation(HalfMove *m) {
+  m->SetIsBlack(IsBlack());
+  m->SetNumber(GetNumber());
+  m->SetParent(this);
+  variations.push_back(m);
+}
 
-    void HalfMove::SetMainline(CMI::HalfMove* m) {
+bool HalfMove::RemoveVariation(HalfMove *m) {
-        mainline = static_cast<HalfMove*>(m);
+  std::vector<HalfMove *> vars;
-        if(m!=nullptr){
+  bool removed = false;
-            if (!this->isBlack) {
+  for (HalfMove *v : GetVariations()) {
-            m->SetIsBlack(true);
+    if (m != v)
-            m->SetNumber(this->number);
+      vars.push_back(v);
-            } else {
+    else
-            m->SetIsBlack(false);
+      removed = true;
-            m->SetNumber(this->number + 1);
+  }
-            }
+  if (removed)
-            m->SetParent(static_cast<CMI::HalfMove*>(this));
+    variations = vars;
-        }
+  return removed;
+}
+
+bool HalfMove::RemoveChild(HalfMove *m) {
+  if (GetMainline() == m) {
+    mainline = nullptr;
+    return true;
+  }
+  return RemoveVariation(m);
+}
+
+bool HalfMove::Contains(HalfMove *m) const {
+  if (m == nullptr)
+    return false;
+  else if (this == m)
+    return true;
+  else if (mainline != nullptr && mainline->Contains(m)) {
+    return true;
+  } else {
+    for (HalfMove *v : variations) {
+      if (v->Contains(m))
+        return true;
     }
+  }
+  return false;
+}
 
-
+bool HalfMove::IsConsistent() const {
-
+  // Check mainline
-    CMI::HalfMove* HalfMove::GetMainline() const {return mainline;};
+  if (mainline != nullptr) {
-    CMI::HalfMove* HalfMove::GetParent() const {return parent;};
+    if (isBlack == mainline->isBlack)
-    std::string HalfMove::GetSAN() const {return SAN;};
+      return false;
-    void HalfMove::SetSAN(std::string newSAN) {SAN=newSAN;};
+    if (isBlack && number + 1 != mainline->number)
-    std::uint16_t HalfMove::GetNumber() const {return number;};
+      return false;
-    void HalfMove::SetNumber(std::uint16_t n) {number=n;};
+    if (!isBlack && number != mainline->number)
-    std::uint8_t HalfMove::GetNAG() const {return NAG;};
+      return false;
-    void HalfMove::SetNAG(std::uint8_t n) {NAG=n;};
+    if (mainline->parent != this)
-    std::string HalfMove::GetComment() const {return comment;};
+      return false;
-    void HalfMove::SetComment(std::string c) { comment=c;};
+    if (!mainline->IsConsistent())
-    bool HalfMove::IsBlack() const {return isBlack;};
+      return false;
-    void HalfMove::SetIsBlack(bool b) {isBlack=b;};
+  }
-
+  // Check variations
-
+  for (HalfMove *v : variations) {
-
+    if (number != v->number || isBlack != v->isBlack || v->parent != this)
-
+      return false;
-
+    if (!v->IsConsistent())
-
+      return false;
-  // ---------- Implementation of various common operations ----------
+  }
-
+  return true;
-    void HalfMove::Promote(){
+}
-        HalfMove *broot=GetBranchRoot();
+} // namespace CMI
-        if(broot!=nullptr){
-            HalfMove *parent=broot->GetParent();
-            if (parent != nullptr) {
-                if (parent->GetMainline() != broot) {
-                    HalfMove *pparent=parent->GetParent();
-                    // First update parent of parent:
-                    if (pparent != nullptr) {
-                        if (pparent->GetMainline() == parent)
-                            pparent->SetMainline(broot);
-                        else { 
-                            pparent->AddVariation(broot);
-                            pparent->RemoveChild(parent);
-                        }
-                    }
-                    // Now update parent:
-                    parent->RemoveChild(broot);
-                    broot->AddVariation(parent);
-                }
-            }
-        }
-    }
-
-    void HalfMove::SetAsMainline(){
-        HalfMove *broot = GetBranchRoot();
-        HalfMove *lastRoot;
-        // Just promote until we cannot anymore
-        do {
-            lastRoot = broot;
-            broot->Promote();
-            broot = GetBranchRoot();
-        } while (broot != lastRoot);
-    }
-
-    HalfMove* HalfMove::GetBranchRoot(){
-        HalfMove *m = this;
-        HalfMove *p = GetParent();
-        while (p != nullptr) {
-            if (p->GetMainline() != m) {
-                return (m);
-            }
-            m = p;
-            p = m->GetParent();
-        }
-        return m;
-    }
-
-    void HalfMove::AddVariation(HalfMove* m){
-        m->SetIsBlack(IsBlack());
-        m->SetNumber(GetNumber());
-        m->SetParent(this);
-        auto vars=GetVariations();
-        vars.push_back(m);
-        SetVariations(vars);
-    }
-
-    bool HalfMove::RemoveVariation(HalfMove* m){
-        std::vector<HalfMove*> vars;
-        bool removed=false;
-        for(HalfMove *v: GetVariations()){
-            if(m!=v)
-                vars.push_back(v);
-            else
-                removed=true; 
-        }
-        if(removed)
-            SetVariations(vars);
-        return removed;
-    }
-
-    bool HalfMove::RemoveChild(HalfMove* m){
-        if(GetMainline()==m){
-            SetMainline(nullptr);
-            return true;
-        }
-        return RemoveVariation(m);
-    }
-}

src/CMI.hpp

@@ -1,61 +1,53 @@
 #pragma once
 
 #include <cstdint>
-#include <vector>
 #include <string>
+#include <vector>
 
 namespace CMI {
 /**
  * @brief Chess Move Interface
  * A standard chess half move interface for improving chess libraries
  * interoperability.
- * NONE OF THESE METHODS IMPLEMENTATIONS MUST DELETE A CMI::HalfMove. 
+ * NONE OF THESE METHODS IMPLEMENTATIONS DELETE A CMI::HalfMove EXCEPT
- * It is up to the user of the object instance to do it after ensuring that no references
+ * CMI::HalfMove::~HalfMove. It is up to the user of the object instance to do
- * to the CMI::HalfMove remains in the move tree.
+ * it after ensuring that no references to the CMI::HalfMove remains in the move
+ * tree.
  */
 class HalfMove {
-protected:
+
-  HalfMove *parent = nullptr;
+  HalfMove *parent;
-  HalfMove *mainline = nullptr;
+  HalfMove *mainline;
   std::vector<HalfMove *> variations;
-  std::string SAN,comment;
+  std::string SAN, comment;
   std::uint16_t number;
   std::uint8_t NAG;
   bool isBlack;
+
+  /// @brief Set the parent of current CMI::HalfMove, only used internally
+  void SetParent(HalfMove *);
+
 public:
   HalfMove();
+  HalfMove(const std::string &SAN);
+  HalfMove(const std::string &SAN, const std::string &comment,
+           std::uint16_t number, std::uint8_t NAG, bool isBlack);
   /// @brief Ensure that the destructor of the child class is called
   virtual ~HalfMove();
-  /**
+  /// @brief  Return a pointer to the next CMI::HalfMove
-    * @brief Return a pointer to the next CMI::HalfMove
+  virtual HalfMove *GetMainline() const;
-    * 
+  /// @brief Bind two moves together
-    * @return HalfMove* if any and nullptr otherwise
+  virtual void SetMainline(HalfMove *);
-    */
+  /// @brief Get the previous CMI::HalfMove
-  virtual HalfMove* GetMainline() const;
+  virtual HalfMove *GetParent() const;
-  /**
-    * @brief Set the next CMI::HalfMove
-    * Existing main line pointer will be overriten (NOT DELETED) and the internal state (Number, IsBlack) of the new move
-    * must be ajusted in the implementation of this method.
-    */
-  virtual void SetMainline(HalfMove*);
-  /**
-    * @brief Get the previous CMI::HalfMove
-    * 
-    * @return HalfMove* if any and nullptr otherwise
-    */
-  virtual HalfMove* GetParent() const;
-  /**
-    * @brief Set the parent of current CMI::HalfMove
-    * 
-    */
-  virtual void SetParent(HalfMove*);
   /// @brief Return the current move using the SAN notation e.g: "Qxc5+" or "a4"
   virtual std::string GetSAN() const;
-  /// @brief Setter to replace current SAN 
+  /// @brief Setter to replace current SAN
-  virtual void SetSAN(std::string);
+  virtual void SetSAN(const std::string &);
-  /// @brief Return the HalfMove move number e.g 1 for the first white's and black's move
+  /// @brief Return the HalfMove move number e.g 1 for the first white's and
+  /// black's move
   virtual std::uint16_t GetNumber() const;
-  /// @brief Setter to replace current Number 
+  /// @brief Setter to replace current Number
   virtual void SetNumber(std::uint16_t);
   /// @brief Return the Numeric Annotation Glyphs code
   virtual std::uint8_t GetNAG() const;
@@ -64,44 +56,40 @@ public:
   /// @brief Return the comment linked to the current move or empty string
   virtual std::string GetComment() const;
   /// @brief Setter to replace current comment
-  virtual void SetComment(std::string);
+  virtual void SetComment(const std::string &);
   /// @brief Return true if the current HalfMove was played by black
   virtual bool IsBlack() const;
-  /// @brief Setter to replace that determined the return values of HalfMove::IsBlack()
+  /// @brief Setter to replace that determined the return values of
+  /// HalfMove::IsBlack()
   virtual void SetIsBlack(bool);
   /// @brief All the variations of the current move
-  virtual std::vector<HalfMove*> GetVariations() const;
+  virtual std::vector<HalfMove *> GetVariations() const;
-  /// @brief Setter to replace current variations
-  virtual void SetVariations(std::vector<HalfMove*>);
-
-
-  // ---------- Implementation of various common operations ----------
-
-  /// @brief Promote the current variation if any
-  void Promote();
-  /// @brief Make the current variation the main line
-  void SetAsMainline();
   /// @brief Add a variation to the variations list
-  void AddVariation(HalfMove*);
+  virtual void AddVariation(HalfMove *);
   /**
-    * @brief Remove the given CMI::HalfMove from the variations list
+   * @brief Remove the given CMI::HalfMove from the variations list
-    * 
+   *
-    * @return true if found and deleted
+   * @return true if found and deleted
-    * @return false otherwise
+   * @return false otherwise
-    */
+   */
-  bool RemoveVariation(HalfMove*);
+  virtual bool RemoveVariation(HalfMove *);
+  virtual void ClearVariations();
   /**
-    * @brief Remove the given CMI::HalfMove from mainline and variations list
+   * @brief Remove the given CMI::HalfMove from mainline and variations list
-    * 
+   *
-    * @return true if found and deleted
+   * @return true if found and deleted
-    * @return false otherwise
+   * @return false otherwise
-    */
+   */
-  bool RemoveChild(HalfMove*);
+  virtual bool RemoveChild(HalfMove *);
-  /**
+  /// @brief Return the CMI::HalfMove root node of the current move branch
-    * @brief Return the CMI::HalfMove root node of the current move branch
+  virtual HalfMove *GetBranchRoot();
-    * 
+  /// @brief Promote the current variation if any
-    * @return HalfMove* the branch root (might be nullptr)
+  virtual void Promote();
-    */
+  /// @brief Make the current variation the main line
-  HalfMove* GetBranchRoot();
+  virtual void SetAsMainline();
+  /// @brief Check if the tree move contains a pointer to the given move
+  virtual bool Contains(HalfMove *) const;
+  /// @brief Check if the current tree of move is consistent
+  virtual bool IsConsistent() const;
 };
-}
+} // namespace CMI

tests/cmi_tests.cpp

@@ -3,21 +3,80 @@
 
 using namespace CMI;
 
-#define NEW_MOVE(VAR,SAN) HalfMove *(VAR)=new HalfMove();  (VAR)->SetSAN((SAN));
+#define NEW_MOVE(VAR, SAN)                                                     \
+  HalfMove *(VAR) = new HalfMove();                                            \
+  (VAR)->SetSAN((SAN));
 
-
+HalfMove *BuildTree() {
-HalfMove *BuildTree(){
+  // Move 1
-    NEW_MOVE(m1,"e4");
+  NEW_MOVE(m1, "e4");
-    NEW_MOVE(m2,"e5");
+  NEW_MOVE(m2, "e5");
-    m1->SetMainline(m2);
+  m1->SetMainline(m2);
-    return m1;
+  // Move 2
+  NEW_MOVE(m3, "Nf3");
+  NEW_MOVE(m4, "Nc6");
+  m2->SetMainline(m3);
+  m3->SetMainline(m4);
+  // Move 3 Ponziani :D
+  NEW_MOVE(m5, "c3");
+  NEW_MOVE(m6, "Nf6");
+  m4->SetMainline(m5);
+  m5->SetMainline(m6);
+  // Move 4 Ponziani :D
+  NEW_MOVE(m7, "d4");
+  NEW_MOVE(m8, "exd4");
+  m5->SetMainline(m7);
+  m7->SetMainline(m8);
+  // Move 4 Variation
+  NEW_MOVE(m8bis, "Nxe4");
+  m8->AddVariation(m8bis);
+  return m1;
 }
 
+TEST_CASE("CMI Tests Numbers", "[numbers]") {
+  HalfMove *m = BuildTree();
+  CHECK(m->IsConsistent());
 
-TEST_CASE("CMI Tests", "[valid]") {
+  CHECK(m->GetNumber() == 1);
-  HalfMove *m=BuildTree();
+  CHECK(m->GetMainline()->GetNumber() == 1);
+  CHECK(m->GetMainline()->GetMainline()->GetNumber() == 2);
+}
 
-CHECK(m->GetNumber()==1);
+TEST_CASE("CMI Tests Consistancy", "[IsConsistent/bindorder]") {
-CHECK(m->GetMainline()->GetNumber()==1);
+  // In mainline
+  NEW_MOVE(m1, "e4");
+  NEW_MOVE(m2, "e5");
+  m1->SetMainline(m2);
+  NEW_MOVE(m3, "Nf3");
+  NEW_MOVE(m4, "Nc6");
+  m3->SetMainline(m4); // Should normally be done after next line
+  m2->SetMainline(m3);
+  CHECK(!m1->IsConsistent());
 
+  // In variations
+  NEW_MOVE(n1, "e4");
+  NEW_MOVE(n2, "e5");
+  n1->SetMainline(n2);
+  NEW_MOVE(n3, "c6");
+  n2->AddVariation(n3);
+  n3->SetNumber(4);
+  CHECK(!n1->IsConsistent());
+}
+
+TEST_CASE("CMI Tests Consistancy", "[IsConsistent/isBlack]") {
+  // In mainline
+  NEW_MOVE(m1, "e4");
+  NEW_MOVE(m2, "e5");
+  m1->SetMainline(m2);
+  m2->SetIsBlack(false);
+  CHECK(!m1->IsConsistent());
+
+  // In variation
+  NEW_MOVE(n1, "e4");
+  NEW_MOVE(n2, "e5");
+  n1->SetMainline(n2);
+  NEW_MOVE(n3, "c6");
+  n2->AddVariation(n3);
+  n3->SetIsBlack(false);
+  CHECK(!n1->IsConsistent());
 }