Clean up day 18

day18/Makefile

@@ -1,7 +1,7 @@
 CC=g++
 BIN_NAME=day18
 CCFLAGS=-o $(BIN_NAME) -g -std=c++17
-LDFLAGS=-lfolly
+LDFLAGS=
 
 .PHONY: all, clean
 

day18/day18.cpp

@@ -1,35 +1,50 @@
-#include <folly/String.h>
-
-#include <cassert>
 #include <fstream>
+#include <functional>
 #include <iostream>
-#include <map>
 #include <memory>
 #include <numeric>
 #include <optional>
-#include <regex>
-#include <set>
 #include <string>
 #include <vector>
 
 enum Operation { ADDITION = '+', MULTIPLICATION = '*', IDENTITY = 'i' };
 class ExpressionNode;
+// A strategy to evaluate the value of a node. Takes the left child, the right child, and the current operation at the
+// node.
 using EvaluationStrategy =
 	std::function<double(const std::unique_ptr<ExpressionNode> &, const std::unique_ptr<ExpressionNode> &, Operation)>;
 
+/**
+ * Represents an abstract node in an expression tree
+ */
 class ExpressionNode {
  public:
 	ExpressionNode() {
 	}
 
+	/**
+	 * Get the current node's value
+	 * @return long The value of the node
+	 */
 	virtual long evaluate() const = 0;
+	/**
+	 * Get references to all of the children to this node
+	 * @return std::vector<std::reference_wrapper<const ExpressionNode>>
+	 */
 	virtual std::vector<std::reference_wrapper<const ExpressionNode>> getChildren() const = 0;
+	/**
+	 * Get the operation of this node
+	 * @return Operation The operation of this node
+	 */
 	virtual Operation getOperation() const = 0;
 
 	virtual ~ExpressionNode() {
 	}
 };
 
+/**
+ * Represents a node that only holds a value
+ */
 class ValueNode : public ExpressionNode {
  public:
 	ValueNode(long n) : n(n) {
@@ -51,9 +66,12 @@ class ValueNode : public ExpressionNode {
 	long n;
 };
 
+/**
+ * Represents a node that can hold a more complex operation (which really ends up being a tree)
+ */
 class ExpressionTree : public ExpressionNode {
  public:
-	ExpressionTree(EvaluationStrategy strategy) : strategy(strategy), isParenthesized(false) {
+	ExpressionTree(EvaluationStrategy strategy) : strategy(strategy) {
 	}
 
 	void setLeft(std::unique_ptr<ExpressionNode> &&left) {
@@ -64,26 +82,13 @@ class ExpressionTree : public ExpressionNode {
 		this->right = std::move(left);
 	}
 
-	void setIsParenthesized(bool isParenthesized) {
-		this->isParenthesized = isParenthesized;
-	}
-
-	void putNextNode(std::unique_ptr<ExpressionNode> &&node) {
-		if (!this->left) {
-			this->setLeft(std::move(node));
-		} else if (!this->right) {
-			this->setRight(std::move(node));
-		} else {
-			std::invalid_argument("Node is already full");
-		}
-	}
-
 	void setOp(Operation op) {
 		this->op = op;
 	}
 
 	long evaluate() const override {
 		if (!this->canFullyEvaluate()) {
+			// It USUALLY shouldn't happen but sometimes parentheticals can only have one child
 			if (this->left) {
 				return this->left->evaluate();
 			} else if (this->right) {
@@ -99,22 +104,18 @@ class ExpressionTree : public ExpressionNode {
 	std::vector<std::reference_wrapper<const ExpressionNode>> getChildren() const override {
 		if (!this->left && !this->right) {
 			throw "Cannot evaluate empty tree's children";
-		} else if (this->left && this->right) {
-			return std::vector<std::reference_wrapper<const ExpressionNode>>{
-				*this->left,
-				*this->right,
-			};
-		} else if (this->left) {
-			return std::vector<std::reference_wrapper<const ExpressionNode>>{
-				*this->left,
-			};
-		} else if (this->right) {
-			return std::vector<std::reference_wrapper<const ExpressionNode>>{
-				*this->right,
-			};
-		} else {
-			throw "Invalid state";
 		}
+
+		std::vector<std::reference_wrapper<const ExpressionNode>> children;
+		// It USUALLY shouldn't happen but sometimes parentheticals can only have one child
+		if (this->left) {
+			children.push_back(*this->left);
+		}
+		if (this->right) {
+			children.push_back(*this->right);
+		}
+
+		return children;
 	}
 
 	Operation getOperation() const {
@@ -129,7 +130,6 @@ class ExpressionTree : public ExpressionNode {
 	std::unique_ptr<ExpressionNode> left;
 	std::unique_ptr<ExpressionNode> right;
 	std::optional<Operation> op;
-	bool isParenthesized;
 	EvaluationStrategy strategy;
 
 	bool canFullyEvaluate() const {
@@ -148,6 +148,11 @@ std::vector<std::string> readInput(const std::string &filename) {
 	return input;
 }
 
+/**
+ * Parse a string component into an operator
+ * @param component The string component to check
+ * @return std::optional<Operation> The operation, if this component represents one. If not, returns an empty optional.
+ */
 std::optional<Operation> parseOperator(const std::string_view component) {
 	if (component.size() != 1) {
 		return std::nullopt;
@@ -163,6 +168,11 @@ std::optional<Operation> parseOperator(const std::string_view component) {
 	}
 }
 
+/**
+ * Parse a string component into a number
+ * @param component The string component to check
+ * @return std::optional<Operation> The number, if this component represents one. If not, returns an empty optional.
+ */
 std::optional<long> parseNumber(const std::string_view component) {
 	try {
 		// This will only really ever copy some (usually small) number of digits... I don't consider it a very
@@ -173,6 +183,12 @@ std::optional<long> parseNumber(const std::string_view component) {
 	}
 }
 
+/**
+ * Parse a parenthetical component of an expression
+ * @param cursor The cursor to start searching backwards from
+ * @param input The full input
+ * @return std::optional<std::pair<std::string_view, int>> The parenthetical, if one exists (without parens).
+ */
 std::optional<std::pair<std::string_view, int>> parseParenthetical(int cursor, const std::string_view input) {
 	if (input.size() == 0 || input.at(cursor) != ')') {
 		return std::nullopt;
@@ -197,10 +213,14 @@ std::optional<std::pair<std::string_view, int>> parseParenthetical(int cursor, c
 	return std::pair<std::string_view, int>(input.substr(startPos + 1, cursor - startPos - 1), startPos);
 }
 
-std::unique_ptr<ExpressionNode> buildTree(
+/**
-	const std::string_view input, const EvaluationStrategy &strategy, bool inParenthetical = false, int depth = 0) {
+ * Build a parse tree
+ * @param input The input to build from
+ * @param strategy The evaluation strategy for each of the nodes to use
+ * @return std::unique_ptr<ExpressionNode> The root of the tree
+ */
+std::unique_ptr<ExpressionNode> buildTree(const std::string_view input, const EvaluationStrategy &strategy) {
 	if (std::count_if(input.cbegin(), input.cend(), [](char c) { return c == ' '; }) == 0) {
-		std::cout << std::string(depth, ' ') << "RIGHT: " << input << std::endl;
 		std::optional<long> value = parseNumber(input);
 		if (!value) {
 			throw std::invalid_argument("one-component string is expected to be number");
@@ -210,7 +230,6 @@ std::unique_ptr<ExpressionNode> buildTree(
 	}
 
 	ExpressionTree tree(strategy);
-	tree.setIsParenthesized(inParenthetical);
 	// This should technically be size_type but I need to be able to go before zero
 	int cursor = input.size() - 1;
 	while (cursor > 0) {
@@ -224,8 +243,9 @@ std::unique_ptr<ExpressionNode> buildTree(
 		std::optional<std::pair<std::string_view, int>> parentheticalPart =
 			parseParenthetical(previousSpace - 1, input);
 		if (parentheticalPart) {
-			std::cout << std::string(depth, ' ') << "RIGHT (paren): " << parentheticalPart->first << std::endl;
+			auto parentheticalTree = buildTree(parentheticalPart->first, strategy);
-			auto parentheticalTree = buildTree(parentheticalPart->first, strategy, true, depth + 1);
+			// NOTE: Just like concrete values, parentheticals will *ALWAYS* (except for leaves) be in the right
+			// subtree. This helps with precedence parsing later
 			tree.setRight(std::move(parentheticalTree));
 			cursor = parentheticalPart->second - 2;
 			continue;
@@ -236,8 +256,7 @@ std::unique_ptr<ExpressionNode> buildTree(
 		if (componentOperation) {
 			tree.setOp(*componentOperation);
 			auto rest = input.substr(0, cursor + 1);
-			std::cout << std::string(depth, ' ') << "LEFT: " << component << std::endl;
+			auto rightTree = buildTree(rest, strategy);
-			auto rightTree = buildTree(rest, strategy, false, depth + 1);
 			tree.setLeft(std::move(rightTree));
 			// Once we have found an operator and the operand to the left of it, we're done
 			break;
@@ -248,13 +267,20 @@ std::unique_ptr<ExpressionNode> buildTree(
 		if (!componentValue) {
 			throw std::invalid_argument("Expected number as last possible option");
 		}
-		std::cout << std::string(depth, ' ') << "RIGHT (value): " << component << std::endl;
+		// NOTE: Concrete values will *ALWAYS* (except at the leaf level) be in the right sub-tree. This was not
+		// intentional but will help with precedence later.
 		tree.setRight(std::make_unique<ValueNode>(*componentValue));
 	}
 
 	return std::make_unique<ExpressionTree>(std::move(tree));
 }
 
+/**
+ * Evaluate the puzzle input, evaluating each node according to the given strategy.
+ * @param input The input
+ * @param strategy The strategy to evaluate each node
+ * @return long The puzzle result
+ */
 long run(const std::vector<std::string> &input, const EvaluationStrategy &strategy) {
 	return std::accumulate(input.cbegin(), input.cend(), 0L, [&strategy](long total, const std::string &expression) {
 		auto tree = buildTree(expression, strategy);
@@ -263,24 +289,24 @@ long run(const std::vector<std::string> &input, const EvaluationStrategy &strate
 }
 
 long part1(const std::vector<std::string> &input) {
-	EvaluationStrategy strategy = [](const std::unique_ptr<ExpressionNode> &left,
+	EvaluationStrategy strategy =
-									 const std::unique_ptr<ExpressionNode> &right,
+		[](const std::unique_ptr<ExpressionNode> &left, const std::unique_ptr<ExpressionNode> &right, Operation op) {
-									 Operation op) -> long {
+			long leftValue = left->evaluate();
-		long leftValue = left->evaluate();
+			long rightValue = right->evaluate();
-		long rightValue = right->evaluate();
 
-		switch (op) {
+			switch (op) {
-			case ADDITION:
+				case ADDITION:
-				return leftValue + rightValue;
+					return leftValue + rightValue;
-			case MULTIPLICATION:
+				case MULTIPLICATION:
-				return leftValue * rightValue;
+					return leftValue * rightValue;
-			default:
+				default:
-				throw std::invalid_argument("Invalid operation");
+					throw std::invalid_argument("Invalid operation");
-		}
+			}
-	};
+		};
 
 	return run(input, strategy);
 }
+
 long part2(const std::vector<std::string> &input) {
 	EvaluationStrategy strategy = [](const std::unique_ptr<ExpressionNode> &left,
 									 const std::unique_ptr<ExpressionNode> &right,
@@ -300,6 +326,9 @@ long part2(const std::vector<std::string> &input) {
 			throw std::invalid_argument("Cannot perform an unknown binary operation on children");
 		}
 
+		// We wish to traverse down the righthand children until we hit a multiplication operator
+		// By doing this, we prioritize adding up the operands first, and then we can evaluate the multiplicand after
+		// the fact
 		ExpressionNode const *prevCursor = nullptr;
 		ExpressionNode const *cursor = left.get();
 		long total = right->evaluate();
@@ -312,26 +341,30 @@ long part2(const std::vector<std::string> &input) {
 			cursor = &leftCursorChild;
 		}
 
-		if (cursor->getChildren().size() == 1) {
+		if (cursor->getChildren().size() == 2) {
-			const ExpressionNode &cursorChild = cursor->getChildren().at(0);
+			auto cursorChildren = cursor->getChildren();
-			Operation finalOperation = op;
+			const ExpressionNode &leftCursorChild = cursorChildren.at(0);
-			if (prevCursor) {
+			const ExpressionNode &rightCursorChild = cursorChildren.at(1);
-				finalOperation = prevCursor->getOperation();
+			// Add the right child of the multiplicand (to keep with addition priority), and multiply the multiplicand
-			}
+			// (since we know that we can no longer use addition)
-			if (finalOperation == ADDITION) {
+			return (total + rightCursorChild.evaluate()) * leftCursorChild.evaluate();
-				return total + cursorChild.evaluate();
+		} else if (cursor->getChildren().size() != 1) {
-			} else if (finalOperation == MULTIPLICATION) {
+			throw std::invalid_argument("Only binary and unary operations are supported");
-				return total * cursorChild.evaluate();
-			} else {
-				throw std::invalid_argument("Cannot perform an unknown binary operation on total");
-			}
 		}
 
-		auto cursorChildren = cursor->getChildren();
+		// If we only have one child, we must consider what the operation was that lead to it.
-		const ExpressionNode &leftCursorChild = cursorChildren.at(0);
+		const ExpressionNode &cursorChild = cursor->getChildren().at(0);
-		const ExpressionNode &rightCursorChild = cursorChildren.at(1);
+		Operation finalOperation = op;
-
+		if (prevCursor) {
-		return (total + rightCursorChild.evaluate()) * leftCursorChild.evaluate();
+			finalOperation = prevCursor->getOperation();
+		}
+		if (finalOperation == ADDITION) {
+			return total + cursorChild.evaluate();
+		} else if (finalOperation == MULTIPLICATION) {
+			return total * cursorChild.evaluate();
+		} else {
+			throw std::invalid_argument("Cannot perform an unknown binary operation on total");
+		}
 	};
 
 	return run(input, strategy);
@@ -345,6 +378,6 @@ int main(int argc, char *argv[]) {
 
 	auto input = readInput(argv[1]);
 
-	// std::cout << part1(input) << std::endl;
+	std::cout << part1(input) << std::endl;
 	std::cout << part2(input) << std::endl;
 }