feat: expression interface init commit, apply review suggestions.

Author: yingcai-cy

src/iceberg/expression.cc

@@ -25,29 +25,29 @@ namespace iceberg {
 
 // True implementation
 const std::shared_ptr<True>& True::Instance() {
-  static const std::shared_ptr<True> instance = std::shared_ptr<True>(new True());
+  static const std::shared_ptr<True> instance{new True()};
   return instance;
 }
 
-Result<ExpressionPtr> True::Negate() const { return False::Instance(); }
+Result<std::shared_ptr<Expression>> True::Negate() const { return False::Instance(); }
 
 // False implementation
 const std::shared_ptr<False>& False::Instance() {
   static const std::shared_ptr<False> instance = std::shared_ptr<False>(new False());
   return instance;
 }
 
-Result<ExpressionPtr> False::Negate() const { return True::Instance(); }
+Result<std::shared_ptr<Expression>> False::Negate() const { return True::Instance(); }
 
 // And implementation
-And::And(ExpressionPtr left, ExpressionPtr right)
+And::And(std::shared_ptr<Expression> left, std::shared_ptr<Expression> right)
     : left_(std::move(left)), right_(std::move(right)) {}
 
 std::string And::ToString() const {
   return std::format("({} and {})", left_->ToString(), right_->ToString());
 }
 
-Result<ExpressionPtr> And::Negate() const {
+Result<std::shared_ptr<Expression>> And::Negate() const {
   // TODO(yingcai-cy): Implement Or expression
   return unexpected(
       Error(ErrorKind::kInvalidExpression, "And negation not yet implemented"));

src/iceberg/expression.h

@@ -31,10 +31,6 @@
 
 namespace iceberg {
 
-/// \brief Type alias for shared pointer to Expression
-class Expression;
-using ExpressionPtr = std::shared_ptr<Expression>;
-
 /// \brief Represents a boolean expression tree.
 class ICEBERG_EXPORT Expression {
  public:
@@ -71,7 +67,7 @@ class ICEBERG_EXPORT Expression {
   virtual Operation op() const = 0;
 
   /// \brief Returns the negation of this expression, equivalent to not(this).
-  virtual Result<ExpressionPtr> Negate() const {
+  virtual Result<std::shared_ptr<Expression>> Negate() const {
     return unexpected(
         Error(ErrorKind::kInvalidExpression, "Expression cannot be negated"));
   }
@@ -99,7 +95,7 @@ class ICEBERG_EXPORT True : public Expression {
 
   std::string ToString() const override { return "true"; }
 
-  Result<ExpressionPtr> Negate() const override;
+  Result<std::shared_ptr<Expression>> Negate() const override;
 
   bool Equals(const Expression& other) const override {
     return other.op() == Operation::kTrue;
@@ -119,7 +115,7 @@ class ICEBERG_EXPORT False : public Expression {
 
   std::string ToString() const override { return "false"; }
 
-  Result<ExpressionPtr> Negate() const override;
+  Result<std::shared_ptr<Expression>> Negate() const override;
 
   bool Equals(const Expression& other) const override {
     return other.op() == Operation::kFalse;
@@ -137,31 +133,31 @@ class ICEBERG_EXPORT And : public Expression {
  public:
   /// \brief Constructs an And expression from two sub-expressions.
   ///
-  /// @param left The left operand of the AND expression
-  /// @param right The right operand of the AND expression
-  And(ExpressionPtr left, ExpressionPtr right);
+  /// \param left The left operand of the AND expression
+  /// \param right The right operand of the AND expression
+  And(std::shared_ptr<Expression> left, std::shared_ptr<Expression> right);
 
   /// \brief Returns the left operand of the AND expression.
   ///
-  /// @return The left operand of the AND expression
-  const ExpressionPtr& left() const { return left_; }
+  /// \return The left operand of the AND expression
+  const std::shared_ptr<Expression>& left() const { return left_; }
 
   /// \brief Returns the right operand of the AND expression.
   ///
-  /// @return The right operand of the AND expression
-  const ExpressionPtr& right() const { return right_; }
+  /// \return The right operand of the AND expression
+  const std::shared_ptr<Expression>& right() const { return right_; }
 
   Operation op() const override { return Operation::kAnd; }
 
   std::string ToString() const override;
 
-  Result<ExpressionPtr> Negate() const override;
+  Result<std::shared_ptr<Expression>> Negate() const override;
 
   bool Equals(const Expression& other) const override;
 
  private:
-  ExpressionPtr left_;
-  ExpressionPtr right_;
+  std::shared_ptr<Expression> left_;
+  std::shared_ptr<Expression> right_;
 };
 
 }  // namespace iceberg

test/expression_test.cc

@@ -25,27 +25,27 @@
 
 namespace iceberg {
 
-TEST(TrueFalseTest, Basi) {
+TEST(TrueFalseTest, Basic) {
   // Test negation of False returns True
-  const std::shared_ptr<False> false_instance = False::Instance();
+  auto false_instance = False::Instance();
   auto negated = false_instance->Negate();
 
   EXPECT_TRUE(negated.has_value());
 
   // Check that negated expression is True
-  std::shared_ptr<Expression> true_expr = negated.value();
+  auto true_expr = negated.value();
   EXPECT_EQ(true_expr->op(), Expression::Operation::kTrue);
 
   EXPECT_EQ(true_expr->ToString(), "true");
 
   // Test negation of True returns false
-  const std::shared_ptr<True> true_instance = True::Instance();
+  auto true_instance = True::Instance();
   negated = true_instance->Negate();
 
   EXPECT_TRUE(negated.has_value());
 
-  // Check that negated expression is True
-  std::shared_ptr<Expression> false_expr = negated.value();
+  // Check that negated expression is False
+  auto false_expr = negated.value();
   EXPECT_EQ(false_expr->op(), Expression::Operation::kFalse);
 
   EXPECT_EQ(false_expr->ToString(), "false");