Skip to content

[REFACTOR] Consolidate query builders with Strategy pattern - Kent Beck Simple Design #310

Description

@kcenon

Summary

Consolidate 4 separate query builder classes into a unified design using the Strategy pattern, following Kent Beck's Simple Design principles (No Duplication, Fewest Elements).

Current State

File: database/query_builder.h

Existing Classes (4 total)

class sql_query_builder { ... };      // Lines 110-188
class mongodb_query_builder { ... };   // Lines 194-266
class redis_query_builder { ... };     // Lines 277-322
class query_builder { ... };           // Lines 328-368 (Universal)

Universal query_builder internals

class query_builder {
private:
    database_types db_type_;
    std::unique_ptr<sql_query_builder> sql_builder_;
    std::unique_ptr<mongodb_query_builder> mongo_builder_;
    std::unique_ptr<redis_query_builder> redis_builder_;
};

Problem Analysis

Issue Kent Beck Principle Violated
4 classes for 1 concept Fewest Elements
Universal builder contains 3 other builders No Duplication
Users confused which builder to use Reveals Intention
Same interface patterns repeated DRY

YAGNI Score: 3/10

  • Individual builders only used by universal builder
  • Most users should only need universal builder
  • Backend-specific APIs rarely used directly

Proposed Solution: Strategy Pattern

New Architecture

namespace database {

// Strategy interface
class query_dialect {
public:
    virtual ~query_dialect() = default;
    virtual std::string format_select(const std::vector<std::string>& cols) const = 0;
    virtual std::string format_from(const std::string& table) const = 0;
    virtual std::string format_where(const query_condition& cond) const = 0;
    virtual std::string format_value(const database_value& val) const = 0;
    virtual std::string build() const = 0;
};

// Concrete strategies (internal)
namespace detail {
    class sql_dialect final : public query_dialect { ... };
    class mongodb_dialect final : public query_dialect { ... };
    class redis_dialect final : public query_dialect { ... };
}

// Unified builder
class query_builder {
public:
    explicit query_builder(database_types db_type);
    
    // Fluent interface
    query_builder& select(const std::vector<std::string>& columns);
    query_builder& from(const std::string& table);
    query_builder& where(const std::string& field, const std::string& op, 
                         const database_value& value);
    
    std::string build() const;
    
private:
    std::unique_ptr<query_dialect> dialect_;  // Only ONE allocated
};

} // namespace database

Factory for dialect creation

std::unique_ptr<query_dialect> create_dialect(database_types type) {
    switch (type) {
        case database_types::postgresql:
        case database_types::mysql:
        case database_types::sqlite:
            return std::make_unique<detail::sql_dialect>(type);
        case database_types::mongodb:
            return std::make_unique<detail::mongodb_dialect>();
        case database_types::redis:
            return std::make_unique<detail::redis_dialect>();
        default:
            throw std::invalid_argument("Unsupported database type");
    }
}

Migration Path

Phase 1: Deprecate individual builders

class [[deprecated("Use query_builder instead")]] sql_query_builder { ... };
class [[deprecated("Use query_builder instead")]] mongodb_query_builder { ... };
class [[deprecated("Use query_builder instead")]] redis_query_builder { ... };

Phase 2: Refactor query_builder internals

  • Replace 3 unique_ptr with 1 dialect unique_ptr
  • Move individual builder logic to dialect implementations

Phase 3: Remove deprecated classes

  • After 1-2 releases, remove sql/mongodb/redis_query_builder

Tasks

  • Create query_dialect interface
  • Implement sql_dialect (extract from sql_query_builder)
  • Implement mongodb_dialect (extract from mongodb_query_builder)
  • Implement redis_dialect (extract from redis_query_builder)
  • Refactor query_builder to use single dialect
  • Add [[deprecated]] to individual builders
  • Update all tests
  • Update documentation
  • Performance benchmark (memory reduction)

Acceptance Criteria

  • Single query_builder class as primary API
  • Only 1 dialect allocated (vs 3 builders before)
  • No breaking changes to public API
  • All existing tests pass
  • Memory footprint reduced by ~66%
  • Individual builders marked deprecated

Metrics

Metric Before After
Public classes 4 1 (+3 internal)
Memory per builder 3 allocations 1 allocation
Lines of code ~270 ~200 (estimated)

Related

Metadata

Metadata

Assignees

Labels

architectureArchitectural changes and designpriority:highHigh priority issuerefactoringCode refactoring and improvements

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions