From 14afb771451f24cf998c9a283860eb9b062382f5 Mon Sep 17 00:00:00 2001
From: Thomas Huetter <thomas.huetter@stud.sbg.ac.at>
Date: Wed, 17 May 2023 15:21:48 +0200
Subject: [PATCH 1/2] fixed parser; removed parser validation function; added
 exceptions for parser

---
 src/command_line/main.cc                      |  9 --
 src/label/json_label.h                        |  1 +
 src/label/string_label.h                      |  1 +
 src/node/node.h                               |  1 +
 src/parser/bracket_notation_parser.h          |  8 --
 src/parser/bracket_notation_parser_impl.h     | 87 ++++++++++---------
 test/common/to_string_converters.h            | 15 ++++
 test/cted_ub/cted_ub_ted_test.cc              | 24 ++---
 test/lgm_ub/lgm_ub_lb_fill_gaps_test.cc       | 25 +++---
 test/lgm_ub/lgm_ub_lb_mapping_test.cc         | 25 +++---
 test/node/tree_indexer_test.cc                | 22 +++--
 test/parser/CMakeLists.txt                    |  5 ++
 test/parser/parser_labels_test.cc             | 37 +++-----
 test/parser/parser_size_test.cc               | 21 +++--
 test/parser/parser_tokens_test.cc             | 37 ++++----
 test/parser/parser_validation_test.cc         | 56 ++++++++++++
 test/parser/parser_validation_test_data.txt   | 21 +++++
 test/sed_lb/sed_lb_ed_test.cc                 | 24 ++---
 test/ted/ted_test.cc                          | 24 ++---
 ...zet_subproblem_count_depth_pruning_test.cc | 24 ++---
 test/touzet/touzet_td_test.cc                 | 25 +++---
 test/touzet/touzet_ted_test.cc                | 25 +++---
 22 files changed, 304 insertions(+), 213 deletions(-)
 create mode 100644 test/parser/parser_validation_test.cc
 create mode 100644 test/parser/parser_validation_test_data.txt

diff --git a/src/command_line/main.cc b/src/command_line/main.cc
index f496db00..0e1da859 100644
--- a/src/command_line/main.cc
+++ b/src/command_line/main.cc
@@ -62,16 +62,7 @@ int main(int argc, char** argv) {
         return -1;
     }
 
-    if (!bnp.validate_input(source_tree_string)) {
-        std::cerr << "Incorrect format of source tree. Is the number of opening and closing brackets equal?" << std::endl;
-        return -1;
-    }
     const node::Node<Label> source_tree = bnp.parse_single(source_tree_string);
-
-    if (!bnp.validate_input(dest_tree_string)) {
-        std::cerr << "Incorrect format of destination tree. Is the number of opening and closing brackets equal?" << std::endl;
-        return -1;
-    }
     const node::Node<Label> destination_tree = bnp.parse_single(dest_tree_string);
 
   std::cout << "Size of source tree:" << source_tree.get_tree_size() << std::endl;
diff --git a/src/label/json_label.h b/src/label/json_label.h
index 0e2138bd..2a055ff6 100644
--- a/src/label/json_label.h
+++ b/src/label/json_label.h
@@ -42,6 +42,7 @@ namespace label {
 /// During the parsing process, the types keys and values cannot 
 class JSONLabel {
 public:
+  JSONLabel() = default;
   JSONLabel(const std::string& label);
 
   /// Operator overloadings.
diff --git a/src/label/string_label.h b/src/label/string_label.h
index 72f2e54c..b7275f97 100644
--- a/src/label/string_label.h
+++ b/src/label/string_label.h
@@ -49,6 +49,7 @@ namespace label {
 /// one wants to use a specialized cost model.
 class StringLabel {
 public:
+    StringLabel() = default;
     StringLabel(const std::string& label);
 
     /// Operator overloadings.
diff --git a/src/node/node.h b/src/node/node.h
index cd3038c6..718f5efa 100644
--- a/src/node/node.h
+++ b/src/node/node.h
@@ -61,6 +61,7 @@ class Node {
 
 // Member functions
 public:
+  Node() = default;
   Node(ConstReference label);
 
   /// Returns the number of children of this node.
diff --git a/src/parser/bracket_notation_parser.h b/src/parser/bracket_notation_parser.h
index e29a3c60..e3654c9a 100644
--- a/src/parser/bracket_notation_parser.h
+++ b/src/parser/bracket_notation_parser.h
@@ -86,14 +86,6 @@ class BracketNotationParser {
   /// \return Vector with all tokens.
   std::vector<std::string> get_tokens(const std::string& tree_string);
 
-  /// Validates the bracket notation input.
-  ///
-  /// NOTE: This function could be merged with parse_string but this may
-  ///       decrease readability.
-  ///
-  /// \param tree_string Tree in bracket notation.
-  /// \return True if the input is correct and false otherwise.
-  bool validate_input(const std::string& tree_string) const;
 // Member variables
 private:
   /// A stack to store nodes on a path to the root from the current node in the
diff --git a/src/parser/bracket_notation_parser_impl.h b/src/parser/bracket_notation_parser_impl.h
index 1dd8b603..d82e9d18 100644
--- a/src/parser/bracket_notation_parser_impl.h
+++ b/src/parser/bracket_notation_parser_impl.h
@@ -34,6 +34,17 @@ node::Node<Label> BracketNotationParser<Label>::parse_single(
 
   std::vector<std::string> tokens = get_tokens(tree_string);
 
+  if (tree_string.size() < 2) {
+    throw std::runtime_error("PARSER-ERROR: Tree string needs at least two characters.");
+  }
+
+  if (tokens[0] != "{") {
+    throw std::runtime_error("PARSER-ERROR: First character must be an opening bracket.");
+  }
+
+  // Metadata for parser to verify the input.
+  int open_nodes = 1;
+
   // Tokenize the input string - get iterator over tokens.
   auto tokens_begin = tokens.begin();
   auto tokens_end = tokens.end();
@@ -41,9 +52,14 @@ node::Node<Label> BracketNotationParser<Label>::parse_single(
   // Deal with the root node separately.
   ++tokens_begin; // Advance tokens to label.
   std::string match_str = *tokens_begin;
-  if (match_str == kLeftBracket || match_str == kRightBracket) { // Root has an empty label.
+  if (match_str == kLeftBracket) { // Root has an empty label.
     match_str = "";
     // Do not advance tokens - we're already at kLeftBracket or kRightBracket.
+  } else if (match_str == kRightBracket) { // Node has an empty label.
+    --open_nodes;
+    match_str = "";
+    ++tokens_begin; // Advance tokens.
+    // Do not advance tokens - we're already at kLeftBracket or kRightBracket.
   } else { // Non-empty label.
     ++tokens_begin; // Advance tokens.
   }
@@ -51,23 +67,21 @@ node::Node<Label> BracketNotationParser<Label>::parse_single(
   node::Node<Label> root(root_label);
   node_stack.push_back(std::ref(root));
 
-  bool consumed_token = false;
-
   // Iterate all remaining tokens.
   while (tokens_begin != tokens_end) {
     match_str = *tokens_begin;
-    consumed_token = false;
 
     if (match_str == kLeftBracket) { // Enter node.
-      consumed_token = true;
-      ++tokens_begin; // Advance tokens to label.
-      match_str = *tokens_begin;
+      // ++tokens_begin; // Advance tokens to label.
+      match_str = *std::next(tokens_begin,1);
+      ++open_nodes;
 
       if (match_str == kLeftBracket || match_str == kRightBracket) { // Node has an empty label.
         match_str = "";
         // Do not advance tokens - we're already at kLeftBracket or kRightBracket.
       } else { // Non-empty label.
-        ++tokens_begin; // Advance tokens.
+        ++tokens_begin; // Advance tokens to get label.
+        match_str = *tokens_begin;
       }
 
       // Create new node.
@@ -79,18 +93,26 @@ node::Node<Label> BracketNotationParser<Label>::parse_single(
       // Put a reference to just-moved n (last child of its parent) on a stack.
       node_stack.push_back(std::ref(node_stack.back().get().add_child(n)));
     }
-
-    if (match_str == kRightBracket) { // Exit node.
-      consumed_token = true;
+    else if (match_str == kRightBracket) { // Exit node.
+      --open_nodes;
+      if (open_nodes < 0) {
+        throw std::runtime_error("PARSER-ERROR: Found a closing bracket for missing opening bracket.");
+      }
       node_stack.pop_back();
-      ++tokens_begin; // Advance tokens.
     }
-
-    // Skip incorrect token.
-    if (consumed_token == false) {
-      ++tokens_begin; // Advance tokens.
+    else {
+      throw std::runtime_error("PARSER-ERROR: Found unexpected token '" + match_str + "'.");
     }
+    ++tokens_begin; // Advance tokens.
   }
+
+  if (open_nodes > 0) {
+    throw std::runtime_error("PARSER-ERROR: There are opening brackets that are never closed.");
+  }
+  if (open_nodes < 0) {
+    throw std::runtime_error("PARSER-ERROR: Found a closing bracket for missing opening bracket.");
+  }
+
   return root;
 }
 
@@ -100,15 +122,17 @@ void BracketNotationParser<Label>::parse_collection(
     const std::string& file_path) {
   std::ifstream trees_file(file_path);
   if (!trees_file) {
-    throw std::runtime_error("ERROR: Problem with opening the file '" + file_path + "' in BracketNotationParser::parse_collection_efficient.");
+    throw std::runtime_error("PARSER-ERROR: Problem with opening the file '" + file_path + "' in BracketNotationParser::parse_collection_efficient.");
   }
   // Read the trees line by line, parse, and move into the container.
   std::string tree_string;
   while (std::getline(trees_file, tree_string)) {
-    if (!validate_input(tree_string)) {
-      continue;
+    try {
+      trees_collection.push_back(parse_single(tree_string)); // -> This invokes a move constructor (due to push_back(<rvalue>)).
+    }
+    catch(const std::exception& e) {
+      std::cout << e.what();
     }
-    trees_collection.push_back(parse_single(tree_string)); // -> This invokes a move constructor (due to push_back(<rvalue>)).
   }
   trees_file.close();
 }
@@ -148,25 +172,4 @@ std::vector<std::string> BracketNotationParser<Label>::get_tokens(
   }
 
   return tokens;
-}
-
-template<class Label>
-bool BracketNotationParser<Label>::validate_input(const std::string& tree_string) const {
-  int bracket_diff_counter = 0; // Counts difference between the numbers of left and right brackets.
-  int bracket_pair_counter = 0; // Counts number of bracket pairs - number of nodes assuming correct nesting.
-  // Loop over all characters.
-  for(auto it = tree_string.begin(); it != tree_string.end(); ++it) {
-    if (*it == kEscapeChar) { // Skip next character if kEscapeChar is found.
-      ++it;
-    } else if (*it == kLeftBracket[0]) { // Increase bracket_counter when kLeftBracket found.
-      bracket_diff_counter++;
-      bracket_pair_counter++;
-    } else if (*it == kRightBracket[0]) { // Decrease bracket_counter when kRightBracket found.
-      bracket_diff_counter--;
-    }
-  }
-  if (bracket_diff_counter != 0 || bracket_pair_counter == 0) {
-    return false;
-  }
-  return true;
-}
+}
\ No newline at end of file
diff --git a/test/common/to_string_converters.h b/test/common/to_string_converters.h
index ba76fa90..463136ff 100644
--- a/test/common/to_string_converters.h
+++ b/test/common/to_string_converters.h
@@ -200,3 +200,18 @@ const std::string map_to_string(const std::unordered_map<int, std::vector<int>>&
 }
 
 }
+
+/// Convert vector of int values to is string representation.
+///
+/// TODO: Move this method to some util.
+///
+/// \param v Vector of int values.
+/// \return String representation of v.
+const std::string vector_to_string(const std::vector<std::string>& v) {
+  std::string s;
+  for (auto e : v) {
+    s += e + ",";
+  }
+  s.pop_back(); // Delete the last coma.s
+  return s;
+}
\ No newline at end of file
diff --git a/test/cted_ub/cted_ub_ted_test.cc b/test/cted_ub/cted_ub_ted_test.cc
index 9f37c7be..eeed47d3 100644
--- a/test/cted_ub/cted_ub_ted_test.cc
+++ b/test/cted_ub/cted_ub_ted_test.cc
@@ -14,6 +14,13 @@ int main() {
   using Label = label::StringLabel;
   using CostModel = cost_model::UnitCostModelLD<Label>;
   using LabelDictionary = label::LabelDictionary<Label>;
+
+  // Declare parser.
+  parser::BracketNotationParser<Label> bnp;
+
+  // Trees.
+  node::Node<Label> t1;
+  node::Node<Label> t2;
   
   // Initialise label dictionary - separate dictionary for each test tree
   // becuse it is easier to keep track of label ids.
@@ -49,20 +56,15 @@ int main() {
       std::getline(test_cases_file, line);
       double correct_result = std::stod(line);
 
-      parser::BracketNotationParser<Label> bnp;
-
-      // Validate test trees.
-      if (!bnp.validate_input(input_tree_1_string)) {
-        std::cerr << "Incorrect format of source tree: '" << input_tree_1_string << "'. Is the number of opening and closing brackets equal?" << std::endl;
-        return -1;
+      // Parse test tree.
+      try {
+        t1 = bnp.parse_single(input_tree_1_string);
+        t2 = bnp.parse_single(input_tree_2_string);
       }
-      if (!bnp.validate_input(input_tree_2_string)) {
-        std::cerr << "Incorrect format of destination tree: '" << input_tree_2_string << "'. Is the number of opening and closing brackets equal?" << std::endl;
+      catch(const std::exception& e) {
+        std::cerr << e.what() << std::endl;
         return -1;
       }
-      // Parse test tree.
-      node::Node<Label> t1 = bnp.parse_single(input_tree_1_string);
-      node::Node<Label> t2 = bnp.parse_single(input_tree_2_string);
 
       // Index input trees.
       node::index_tree(ti1, t1, ld, ucm);
diff --git a/test/lgm_ub/lgm_ub_lb_fill_gaps_test.cc b/test/lgm_ub/lgm_ub_lb_fill_gaps_test.cc
index 9fcc2730..ba44e6b7 100644
--- a/test/lgm_ub/lgm_ub_lb_fill_gaps_test.cc
+++ b/test/lgm_ub/lgm_ub_lb_fill_gaps_test.cc
@@ -15,6 +15,13 @@ int main() {
   using Label = label::StringLabel;
   using CostModel = cost_model::UnitCostModelLD<Label>;
   using LabelDictionary = label::LabelDictionary<Label>;
+
+  // Declare parser.
+  parser::BracketNotationParser<Label> bnp;
+
+  // Trees.
+  node::Node<Label> t1;
+  node::Node<Label> t2;
   
   // Initialise label dictionary - separate dictionary for each test tree
   // becuse it is easier to keep track of label ids.
@@ -38,8 +45,6 @@ int main() {
   node::TreeIndexLGM ti1;
   node::TreeIndexLGM ti2;
 
-  parser::BracketNotationParser<Label> bnp;
-
   // Fixed test cases.
   std::cout << "--- FIXED TEST ---" << std::endl;
 
@@ -60,20 +65,16 @@ int main() {
       std::getline(test_cases_file, line);
       std::string correct_result = line;
 
-      // Validate test trees.
-      if (!bnp.validate_input(input_tree_1_string)) {
-        std::cerr << "Incorrect format of source tree: '" << input_tree_1_string << "'. Is the number of opening and closing brackets equal?" << std::endl;
-        return -1;
+      // Parse test tree.
+      try {
+        t1 = bnp.parse_single(input_tree_1_string);
+        t2 = bnp.parse_single(input_tree_2_string);
       }
-      if (!bnp.validate_input(input_tree_2_string)) {
-        std::cerr << "Incorrect format of destination tree: '" << input_tree_2_string << "'. Is the number of opening and closing brackets equal?" << std::endl;
+      catch(const std::exception& e) {
+        std::cerr << e.what() << std::endl;
         return -1;
       }
 
-      // Parse test tree.
-      node::Node<Label> t1 = bnp.parse_single(input_tree_1_string);
-      node::Node<Label> t2 = bnp.parse_single(input_tree_2_string);
-
       // Index input trees.
       node::index_tree(ti1, t1, ld, ucm);
       node::index_tree(ti2, t2, ld, ucm);
diff --git a/test/lgm_ub/lgm_ub_lb_mapping_test.cc b/test/lgm_ub/lgm_ub_lb_mapping_test.cc
index 65a4018a..57dcc48a 100644
--- a/test/lgm_ub/lgm_ub_lb_mapping_test.cc
+++ b/test/lgm_ub/lgm_ub_lb_mapping_test.cc
@@ -15,6 +15,13 @@ int main() {
   using Label = label::StringLabel;
   using CostModel = cost_model::UnitCostModelLD<Label>;
   using LabelDictionary = label::LabelDictionary<Label>;
+
+  // Declare parser.
+  parser::BracketNotationParser<Label> bnp;
+
+  // Trees.
+  node::Node<Label> t1;
+  node::Node<Label> t2;
   
   // Initialise label dictionary - separate dictionary for each test tree
   // becuse it is easier to keep track of label ids.
@@ -38,8 +45,6 @@ int main() {
   node::TreeIndexLGM ti1;
   node::TreeIndexLGM ti2;
 
-  parser::BracketNotationParser<Label> bnp;
-
   // Fixed test cases.
   std::cout << "--- FIXED TEST ---" << std::endl;
 
@@ -60,20 +65,16 @@ int main() {
       std::getline(test_cases_file, line);
       std::string correct_result = line;
 
-      // Validate test trees.
-      if (!bnp.validate_input(input_tree_1_string)) {
-        std::cerr << "Incorrect format of source tree: '" << input_tree_1_string << "'. Is the number of opening and closing brackets equal?" << std::endl;
-        return -1;
+      // Parse test tree.
+      try {
+        t1 = bnp.parse_single(input_tree_1_string);
+        t2 = bnp.parse_single(input_tree_2_string);
       }
-      if (!bnp.validate_input(input_tree_2_string)) {
-        std::cerr << "Incorrect format of destination tree: '" << input_tree_2_string << "'. Is the number of opening and closing brackets equal?" << std::endl;
+      catch(const std::exception& e) {
+        std::cerr << e.what() << std::endl;
         return -1;
       }
 
-      // Parse test tree.
-      node::Node<Label> t1 = bnp.parse_single(input_tree_1_string);
-      node::Node<Label> t2 = bnp.parse_single(input_tree_2_string);
-
       // Index input trees.
       node::index_tree(ti1, t1, ld, ucm);
       node::index_tree(ti2, t2, ld, ucm);
diff --git a/test/node/tree_indexer_test.cc b/test/node/tree_indexer_test.cc
index 5ec02181..da7fe7dc 100644
--- a/test/node/tree_indexer_test.cc
+++ b/test/node/tree_indexer_test.cc
@@ -134,6 +134,12 @@ int main(int, char** argv) {
   using CostModel = cost_model::UnitCostModelLD<Label>;
   using LabelDictionary = label::LabelDictionary<Label>;
 
+  // Declare parser.
+  parser::BracketNotationParser<Label> bnp;
+
+  // Declare trees.
+  node::Node<Label> tree;
+
   // Parse test cases from file.
   std::ifstream test_data_file(index_test_name + "_data.txt");
   if (!test_data_file.is_open()) {
@@ -149,18 +155,16 @@ int main(int, char** argv) {
       std::string input_tree = line;
       std::getline(test_data_file, line);
       std::string correct_result = line;
-      
-      parser::BracketNotationParser<Label> bnp;
-      
-      // Validate test tree.
-      if (!bnp.validate_input(input_tree)) {
-        std::cerr << "Incorrect format of input tree: '" << input_tree << "'. Is the number of opening and closing brackets equal?" << std::endl;
+
+      // Parse test tree.
+      try {
+        tree = bnp.parse_single(input_tree);
+      }
+      catch(const std::exception& e) {
+        std::cerr << e.what() << std::endl;
         return -1;
       }
       
-      // Parse test tree.
-      node::Node<Label> tree = bnp.parse_single(input_tree);
-      
       // Initialise label dictionary - separate dictionary for each test tree
       // becuse it is easier to keep track of label ids.
       LabelDictionary ld;
diff --git a/test/parser/CMakeLists.txt b/test/parser/CMakeLists.txt
index 3dc1aee5..de3ac96e 100644
--- a/test/parser/CMakeLists.txt
+++ b/test/parser/CMakeLists.txt
@@ -6,6 +6,7 @@ set(parser_test_list
   parser_labels_test          # Testing parsed labels.
   parser_tokens_test          # Testing parsed tokens.
   parser_collection_size_test # Testing collection tree sizes after parsing (reading from file).
+  parser_validation_test      # Testing the validation function of the parser.
 )
 
 # Add custom target for building all parser's tests.
@@ -25,6 +26,10 @@ foreach(loop_var ${parser_test_list})
     ${loop_var}_driver # EXECUTABLE NAME
     TreeSimilarity     # LIBRARY NAME
   )
+  target_link_libraries(
+    ${loop_var}_driver # EXECUTABLE NAME
+    TestCommon         # LIBRARY NAME
+  )
   add_test(
     NAME    ${loop_var}        # TEST NAME
     COMMAND ${loop_var}_driver # EXECUTABLE NAME
diff --git a/test/parser/parser_labels_test.cc b/test/parser/parser_labels_test.cc
index 9b7efb4f..deb50087 100644
--- a/test/parser/parser_labels_test.cc
+++ b/test/parser/parser_labels_test.cc
@@ -5,26 +5,17 @@
 #include "string_label.h"
 #include "node.h"
 #include "bracket_notation_parser.h"
-
-/// Convert vector of int values to is string representation.
-///
-/// TODO: Move this method to some util.
-///
-/// \param v Vector of int values.
-/// \return String representation of v.
-const std::string vector_to_string(const std::vector<std::string>& v) {
-  std::string s;
-  for (auto e : v) {
-    s += e + ",";
-  }
-  s.pop_back(); // Delete the last coma.s
-  return s;
-}
+#include "to_string_converters.h"
 
 int main() {
 
   using Label = label::StringLabel;
 
+  // Declarations.
+  parser::BracketNotationParser<Label> bnp;
+  std::string computed_results;
+  node::Node<Label> t;
+
   // Parse test cases from file.
   std::ifstream test_cases_file("parser_labels_test_data.txt");
 
@@ -39,17 +30,17 @@ int main() {
       std::getline(test_cases_file, line);
       std::string correct_result = line;
 
-      parser::BracketNotationParser<Label> bnp;
-
-      // Validate test tree.
-      if (!bnp.validate_input(input_tree)) {
-        std::cerr << "Incorrect format of input tree: '" << input_tree << "'. Is the number of opening and closing brackets equal?" << std::endl;
+      // Parse test tree.
+      try {
+        t = bnp.parse_single(input_tree);
+      }
+      catch(const std::exception& e) {
+        std::cerr << e.what() << std::endl;
         return -1;
       }
-      // Parse test tree.
-      node::Node<Label> t = bnp.parse_single(input_tree);
 
-      std::string computed_results = vector_to_string(t.get_all_labels());
+      // Compute label sets as comma separated strings.
+      computed_results = vector_to_string(t.get_all_labels());
 
       if (correct_result != computed_results) {
         std::cerr << "Incorrect labels: " << computed_results << " instead of " << correct_result << std::endl;
diff --git a/test/parser/parser_size_test.cc b/test/parser/parser_size_test.cc
index 9e64b428..6d7f70d3 100644
--- a/test/parser/parser_size_test.cc
+++ b/test/parser/parser_size_test.cc
@@ -10,6 +10,11 @@ int main() {
 
   using Label = label::StringLabel;
 
+  // Declarations.
+  parser::BracketNotationParser<Label> bnp;
+  int computed_results;
+  node::Node<Label> t;
+
   // Parse test cases from file.
   std::ifstream test_cases_file("parser_size_test_data.txt");
 
@@ -24,17 +29,17 @@ int main() {
       std::getline(test_cases_file, line);
       int correct_result = std::stoi(line);
 
-      parser::BracketNotationParser<Label> bnp;
-
-      // Validate test tree.
-      if (!bnp.validate_input(input_tree)) {
-        std::cerr << "Incorrect format of input tree: '" << input_tree << "'. Is the number of opening and closing brackets equal?" << std::endl;
+      // Parse test tree.
+      try {
+        t = bnp.parse_single(input_tree);
+      }
+      catch(const std::exception& e) {
+        std::cerr << e.what() << std::endl;
         return -1;
       }
-      // Parse test tree.
-      node::Node<Label> t = bnp.parse_single(input_tree);
 
-      int computed_results = t.get_tree_size();
+      // Compute tree size as integer.
+      computed_results = t.get_tree_size();
 
       if (correct_result != computed_results) {
         std::cerr << "Incorrect number of nodes: " << computed_results << " instead of " << correct_result << std::endl;
diff --git a/test/parser/parser_tokens_test.cc b/test/parser/parser_tokens_test.cc
index 9839ca2d..d6a85ac2 100644
--- a/test/parser/parser_tokens_test.cc
+++ b/test/parser/parser_tokens_test.cc
@@ -3,25 +3,16 @@
 #include <vector>
 #include <fstream>
 #include "bracket_notation_parser.h"
-
-/// Convert vector of int values to is string representation.
-///
-/// TODO: Move this method to some util.
-///
-/// \param v Vector of int values.
-/// \return String representation of v.
-const std::string vector_to_string(const std::vector<std::string>& v) {
-  std::string s;
-  for (auto e : v) {
-    s += e + ",";
-  }
-  s.pop_back(); // Delete the last coma
-  return s;
-}
+#include "to_string_converters.h"
 
 int main() {
 
   using Label = label::StringLabel;
+
+  // Declarations.
+  parser::BracketNotationParser<Label> bnp;
+  std::string computed_results;
+  node::Node<Label> t;
   
   std::cout << "Tokens test." << std::endl;
 
@@ -39,15 +30,17 @@ int main() {
       std::getline(test_cases_file, line);
       std::string correct_result = line;
 
-      parser::BracketNotationParser<Label> bnp;
-
-      // Validate test tree.
-      if (!bnp.validate_input(input_tree)) {
-        std::cerr << "Incorrect format of input tree: '" << input_tree << "'. Is the number of opening and closing brackets equal?" << std::endl;
+      // Parse test tree.
+      try {
+        t = bnp.parse_single(input_tree);
+      }
+      catch(const std::exception& e) {
+        std::cerr << e.what() << std::endl;
         return -1;
       }
-      // Parse test tree.
-      std::string computed_results = vector_to_string(bnp.get_tokens(input_tree));
+
+      // Compute label sets as comma separated strings.
+      computed_results = vector_to_string(bnp.get_tokens(input_tree));
 
       if (correct_result != computed_results) {
         std::cerr << "Incorrect tokens: " << computed_results << " instead of " << correct_result << std::endl;
diff --git a/test/parser/parser_validation_test.cc b/test/parser/parser_validation_test.cc
new file mode 100644
index 00000000..ba390016
--- /dev/null
+++ b/test/parser/parser_validation_test.cc
@@ -0,0 +1,56 @@
+#include <iostream>
+#include <string>
+#include <vector>
+#include <fstream>
+#include "bracket_notation_parser.h"
+
+int main() {
+
+  using Label = label::StringLabel;
+  
+  parser::BracketNotationParser<Label> bnp;
+  node::Node<Label> tree;
+  int test_cases = 0;
+  int identified_cases = 0;
+  
+  std::cout << "Validation test." << std::endl;
+
+  // Parse test cases from file.
+  std::ifstream test_cases_file("parser_validation_test_data.txt");
+
+  if (!test_cases_file) {
+    std::cerr << "ERROR: Problem reading the test file." << std::endl;
+  }
+
+  for (std::string line; std::getline(test_cases_file, line);) {
+    if (line[0] == '#') { // Identify first line of test case by '#'.
+      ++test_cases;
+      std::getline(test_cases_file, line);
+      std::string input_tree = line;
+      std::getline(test_cases_file, line);
+      std::string correct_result = line;
+
+      // Parse test trees and check for exception identification.
+      try {
+        tree = bnp.parse_single(input_tree);
+      }
+      catch(const std::exception& e) {
+        std::cerr << e.what() << std::endl;
+        if (correct_result != e.what()) {
+          std::cerr << "Validation test failed: " << e.what() <<
+            " instead of " << correct_result << std::endl;
+          return -1;
+        }
+        ++identified_cases;
+      }
+    }
+  }
+  
+  if (test_cases != identified_cases) {
+    std::cerr << "Validation test failed: only identified " << 
+      identified_cases << " out of " << test_cases << std::endl;
+    return -1;
+  }
+
+  return 0;
+}
diff --git a/test/parser/parser_validation_test_data.txt b/test/parser/parser_validation_test_data.txt
new file mode 100644
index 00000000..73dddb47
--- /dev/null
+++ b/test/parser/parser_validation_test_data.txt
@@ -0,0 +1,21 @@
+# Test case 1
+{{}
+PARSER-ERROR: There are opening brackets that are never closed.
+# Test case 2
+{a{b}c}
+PARSER-ERROR: Found unexpected token 'c'.
+# Test case 3
+}{"a{"b"}}
+PARSER-ERROR: First character must be an opening bracket.
+# Test case 4
+}{"a{"b"}}{
+PARSER-ERROR: First character must be an opening bracket.
+# Test case 5
+{{}}{{}
+PARSER-ERROR: There are opening brackets that are never closed.
+# Test case 6
+{{}}}{
+PARSER-ERROR: Found a closing bracket for missing opening bracket.
+# Test case 7
+{
+PARSER-ERROR: Tree string needs at least two characters.
diff --git a/test/sed_lb/sed_lb_ed_test.cc b/test/sed_lb/sed_lb_ed_test.cc
index 5b6fd493..3ec27c06 100644
--- a/test/sed_lb/sed_lb_ed_test.cc
+++ b/test/sed_lb/sed_lb_ed_test.cc
@@ -14,6 +14,13 @@ int main() {
   using Label = label::StringLabel;
   using CostModel = cost_model::UnitCostModelLD<Label>;
   using LabelDictionary = label::LabelDictionary<Label>;
+
+  // Declare parser.
+  parser::BracketNotationParser<Label> bnp;
+
+  // Trees.
+  node::Node<Label> t1;
+  node::Node<Label> t2;
   
   // Initialise label dictionary - separate dictionary for each test tree
   // becuse it is easier to keep track of label ids.
@@ -48,20 +55,15 @@ int main() {
       std::getline(test_cases_file, line);
       double correct_result = std::stod(line);
 
-      parser::BracketNotationParser<Label> bnp;
-
-      // Validate test trees.
-      if (!bnp.validate_input(input_tree_1_string)) {
-        std::cerr << "Incorrect format of source tree: '" << input_tree_1_string << "'. Is the number of opening and closing brackets equal?" << std::endl;
-        return -1;
+      // Parse test tree.
+      try {
+        t1 = bnp.parse_single(input_tree_1_string);
+        t2 = bnp.parse_single(input_tree_2_string);
       }
-      if (!bnp.validate_input(input_tree_2_string)) {
-        std::cerr << "Incorrect format of destination tree: '" << input_tree_2_string << "'. Is the number of opening and closing brackets equal?" << std::endl;
+      catch(const std::exception& e) {
+        std::cerr << e.what() << std::endl;
         return -1;
       }
-      // Parse test tree.
-      node::Node<Label> t1 = bnp.parse_single(input_tree_1_string);
-      node::Node<Label> t2 = bnp.parse_single(input_tree_2_string);
 
       // Index input trees.
       node::index_tree(ti1, t1, ld, ucm);
diff --git a/test/ted/ted_test.cc b/test/ted/ted_test.cc
index 4bbb0d1d..0d71d481 100644
--- a/test/ted/ted_test.cc
+++ b/test/ted/ted_test.cc
@@ -25,6 +25,13 @@ int main(int, char** argv) {
   using Label = label::StringLabel;
   using CostModel = cost_model::UnitCostModelLD<Label>;
   using LabelDictionary = label::LabelDictionary<Label>;
+
+  // Declare parser.
+  parser::BracketNotationParser<Label> bnp;
+
+  // Trees.
+  node::Node<Label> t1;
+  node::Node<Label> t2;
   
   // Initialise label dictionary - separate dictionary for each test tree
   // becuse it is easier to keep track of label ids.
@@ -104,20 +111,15 @@ int main(int, char** argv) {
       std::getline(test_cases_file, line);
       double correct_result = std::stod(line);
 
-      parser::BracketNotationParser<Label> bnp;
-
-      // Validate test trees.
-      if (!bnp.validate_input(input_tree_1_string)) {
-        std::cerr << "Incorrect format of source tree: '" << input_tree_1_string << "'. Is the number of opening and closing brackets equal?" << std::endl;
-        return -1;
+      // Parse test tree.
+      try {
+        t1 = bnp.parse_single(input_tree_1_string);
+        t2 = bnp.parse_single(input_tree_2_string);
       }
-      if (!bnp.validate_input(input_tree_2_string)) {
-        std::cerr << "Incorrect format of destination tree: '" << input_tree_2_string << "'. Is the number of opening and closing brackets equal?" << std::endl;
+      catch(const std::exception& e) {
+        std::cerr << e.what() << std::endl;
         return -1;
       }
-      // Parse test tree.
-      node::Node<Label> t1 = bnp.parse_single(input_tree_1_string);
-      node::Node<Label> t2 = bnp.parse_single(input_tree_2_string);
       
       // Index input trees.
       node::index_tree(ti1, t1, ld, ucm);
diff --git a/test/touzet/touzet_subproblem_count_depth_pruning_test.cc b/test/touzet/touzet_subproblem_count_depth_pruning_test.cc
index 6ca21a31..efa8849d 100644
--- a/test/touzet/touzet_subproblem_count_depth_pruning_test.cc
+++ b/test/touzet/touzet_subproblem_count_depth_pruning_test.cc
@@ -17,6 +17,13 @@ int main() {
   using Label = label::StringLabel;
   using CostModel = cost_model::UnitCostModelLD<Label>;
   using LabelDictionary = label::LabelDictionary<Label>;
+
+  // Declare parser.
+  parser::BracketNotationParser<Label> bnp;
+
+  // Trees.
+  node::Node<Label> t1;
+  node::Node<Label> t2;
   
   // Initialise label dictionary - separate dictionary for each test tree
   // becuse it is easier to keep track of label ids.
@@ -53,20 +60,15 @@ int main() {
       std::getline(test_cases_file, line);
       unsigned long long int correct_result = std::stoll(line);
 
-      parser::BracketNotationParser<Label> bnp;
-
-      // Validate test trees.
-      if (!bnp.validate_input(input_tree_1_string)) {
-        std::cerr << "Incorrect format of source tree: '" << input_tree_1_string << "'. Is the number of opening and closing brackets equal?" << std::endl;
-        return -1;
+      // Parse test tree.
+      try {
+        t1 = bnp.parse_single(input_tree_1_string);
+        t2 = bnp.parse_single(input_tree_2_string);
       }
-      if (!bnp.validate_input(input_tree_2_string)) {
-        std::cerr << "Incorrect format of destination tree: '" << input_tree_2_string << "'. Is the number of opening and closing brackets equal?" << std::endl;
+      catch(const std::exception& e) {
+        std::cerr << e.what() << std::endl;
         return -1;
       }
-      // Parse test tree.
-      node::Node<Label> t1 = bnp.parse_single(input_tree_1_string);
-      node::Node<Label> t2 = bnp.parse_single(input_tree_2_string);
 
       // Index input trees.
       node::index_tree(ti1, t1, ld, ucm);
diff --git a/test/touzet/touzet_td_test.cc b/test/touzet/touzet_td_test.cc
index 284d93df..6c5397e6 100644
--- a/test/touzet/touzet_td_test.cc
+++ b/test/touzet/touzet_td_test.cc
@@ -20,6 +20,13 @@ int main() {
   using CostModel = cost_model::UnitCostModelLD<Label>;
   using LabelDictionary = label::LabelDictionary<Label>;
   
+  // Declare parser.
+  parser::BracketNotationParser<Label> bnp;
+
+  // Trees.
+  node::Node<Label> t1;
+  node::Node<Label> t2;
+  
   // Initialise label dictionary - separate dictionary for each test tree
   // becuse it is easier to keep track of label ids.
   LabelDictionary ld;
@@ -76,21 +83,15 @@ int main() {
         correct_result.pop_back();
       }
 
-      parser::BracketNotationParser<Label> bnp;
-
-      // Validate test tree.
-      if (!bnp.validate_input(source_tree)) {
-        std::cerr << "Incorrect format of input tree: '" << source_tree << "'. Is the number of opening and closing brackets equal?" << std::endl;
-        return -1;
+      // Parse test tree.
+      try {
+        t1 = bnp.parse_single(source_tree);
+        t2 = bnp.parse_single(destination_tree);
       }
-      // Validate test tree.
-      if (!bnp.validate_input(destination_tree)) {
-        std::cerr << "Incorrect format of input tree: '" << destination_tree << "'. Is the number of opening and closing brackets equal?" << std::endl;
+      catch(const std::exception& e) {
+        std::cerr << e.what() << std::endl;
         return -1;
       }
-      // Parse test tree.
-      node::Node<Label> t1 = bnp.parse_single(source_tree);
-      node::Node<Label> t2 = bnp.parse_single(destination_tree);
 
       // Index input trees.
       node::index_tree(ti1, t1, ld, ucm);
diff --git a/test/touzet/touzet_ted_test.cc b/test/touzet/touzet_ted_test.cc
index 6c1f7d0d..e164632a 100644
--- a/test/touzet/touzet_ted_test.cc
+++ b/test/touzet/touzet_ted_test.cc
@@ -19,6 +19,13 @@ int main() {
   using CostModel = cost_model::UnitCostModelLD<Label>;
   using LabelDictionary = label::LabelDictionary<Label>;
   
+  // Declare parser.
+  parser::BracketNotationParser<Label> bnp;
+
+  // Trees.
+  node::Node<Label> t1;
+  node::Node<Label> t2;
+
   // Initialise label dictionary - separate dictionary for each test tree
   // becuse it is easier to keep track of label ids.
   LabelDictionary ld;
@@ -56,21 +63,15 @@ int main() {
       std::getline(test_cases_file, line);
       double expected_result = std::stod(line);
 
-      parser::BracketNotationParser<Label> bnp;
-
-      // Validate test tree.
-      if (!bnp.validate_input(source_tree)) {
-        std::cerr << "Incorrect format of input tree: '" << source_tree << "'. Is the number of opening and closing brackets equal?" << std::endl;
-        return -1;
+      // Parse test tree.
+      try {
+        t1 = bnp.parse_single(source_tree);
+        t2 = bnp.parse_single(destination_tree);
       }
-      // Validate test tree.
-      if (!bnp.validate_input(destination_tree)) {
-        std::cerr << "Incorrect format of input tree: '" << destination_tree << "'. Is the number of opening and closing brackets equal?" << std::endl;
+      catch(const std::exception& e) {
+        std::cerr << e.what() << std::endl;
         return -1;
       }
-      // Parse test tree.
-      node::Node<Label> t1 = bnp.parse_single(source_tree);
-      node::Node<Label> t2 = bnp.parse_single(destination_tree);
 
       // Index input trees.
       node::index_tree(ti1, t1, ld, ucm);

From d5b7c914b8eb576ba6b6cffe73c487384021e89e Mon Sep 17 00:00:00 2001
From: Thomas Huetter <thomas.huetter@stud.sbg.ac.at>
Date: Mon, 12 Jun 2023 10:22:28 +0200
Subject: [PATCH 2/2] address comments from code review by mateusz

---
 src/parser/bracket_notation_parser_impl.h | 18 +++++++++---------
 test/common/to_string_converters.h        |  2 --
 test/parser/parser_validation_test.cc     |  2 +-
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/src/parser/bracket_notation_parser_impl.h b/src/parser/bracket_notation_parser_impl.h
index d82e9d18..7148acfe 100644
--- a/src/parser/bracket_notation_parser_impl.h
+++ b/src/parser/bracket_notation_parser_impl.h
@@ -32,20 +32,21 @@ template<class Label>
 node::Node<Label> BracketNotationParser<Label>::parse_single(
     const std::string& tree_string) {
 
+  // Tokenize the input string.
   std::vector<std::string> tokens = get_tokens(tree_string);
 
   if (tree_string.size() < 2) {
-    throw std::runtime_error("PARSER-ERROR: Tree string needs at least two characters.");
+    throw std::invalid_argument("PARSER-ERROR: Tree string needs at least two characters.");
   }
 
   if (tokens[0] != "{") {
-    throw std::runtime_error("PARSER-ERROR: First character must be an opening bracket.");
+    throw std::invalid_argument("PARSER-ERROR: First character must be an opening bracket.");
   }
 
   // Metadata for parser to verify the input.
   int open_nodes = 1;
 
-  // Tokenize the input string - get iterator over tokens.
+  // Get iterator over input tokens.
   auto tokens_begin = tokens.begin();
   auto tokens_end = tokens.end();
 
@@ -59,7 +60,6 @@ node::Node<Label> BracketNotationParser<Label>::parse_single(
     --open_nodes;
     match_str = "";
     ++tokens_begin; // Advance tokens.
-    // Do not advance tokens - we're already at kLeftBracket or kRightBracket.
   } else { // Non-empty label.
     ++tokens_begin; // Advance tokens.
   }
@@ -96,21 +96,21 @@ node::Node<Label> BracketNotationParser<Label>::parse_single(
     else if (match_str == kRightBracket) { // Exit node.
       --open_nodes;
       if (open_nodes < 0) {
-        throw std::runtime_error("PARSER-ERROR: Found a closing bracket for missing opening bracket.");
+        throw std::invalid_argument("PARSER-ERROR: Found a closing bracket for missing opening bracket.");
       }
       node_stack.pop_back();
     }
     else {
-      throw std::runtime_error("PARSER-ERROR: Found unexpected token '" + match_str + "'.");
+      throw std::invalid_argument("PARSER-ERROR: Found unexpected token '" + match_str + "'.");
     }
     ++tokens_begin; // Advance tokens.
   }
 
   if (open_nodes > 0) {
-    throw std::runtime_error("PARSER-ERROR: There are opening brackets that are never closed.");
+    throw std::invalid_argument("PARSER-ERROR: There are opening brackets that are never closed.");
   }
   if (open_nodes < 0) {
-    throw std::runtime_error("PARSER-ERROR: Found a closing bracket for missing opening bracket.");
+    throw std::invalid_argument("PARSER-ERROR: Found a closing bracket for missing opening bracket.");
   }
 
   return root;
@@ -122,7 +122,7 @@ void BracketNotationParser<Label>::parse_collection(
     const std::string& file_path) {
   std::ifstream trees_file(file_path);
   if (!trees_file) {
-    throw std::runtime_error("PARSER-ERROR: Problem with opening the file '" + file_path + "' in BracketNotationParser::parse_collection_efficient.");
+    throw std::invalid_argument("PARSER-ERROR: Problem with opening the file '" + file_path + "' in BracketNotationParser::parse_collection_efficient.");
   }
   // Read the trees line by line, parse, and move into the container.
   std::string tree_string;
diff --git a/test/common/to_string_converters.h b/test/common/to_string_converters.h
index 463136ff..40b066a0 100644
--- a/test/common/to_string_converters.h
+++ b/test/common/to_string_converters.h
@@ -203,8 +203,6 @@ const std::string map_to_string(const std::unordered_map<int, std::vector<int>>&
 
 /// Convert vector of int values to is string representation.
 ///
-/// TODO: Move this method to some util.
-///
 /// \param v Vector of int values.
 /// \return String representation of v.
 const std::string vector_to_string(const std::vector<std::string>& v) {
diff --git a/test/parser/parser_validation_test.cc b/test/parser/parser_validation_test.cc
index ba390016..76954e91 100644
--- a/test/parser/parser_validation_test.cc
+++ b/test/parser/parser_validation_test.cc
@@ -47,7 +47,7 @@ int main() {
   }
   
   if (test_cases != identified_cases) {
-    std::cerr << "Validation test failed: only identified " << 
+    std::cerr << "Validation test failed: correctly caught " << 
       identified_cases << " out of " << test_cases << std::endl;
     return -1;
   }