From a5ed0193f6b100933df1d4f89b693141ce59549e Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Sun, 24 May 2026 12:50:29 +0300 Subject: rpn: remove combined Operator interface per ISP The Operator interface (~40 methods) embedded 7 sub-interfaces plus mode/prefix/metric/custom methods. Since RPN is the sole client and *Operations is the sole implementor, the combined interface added indirection without practical benefit. Changes: - Remove type Operator entirely - RPN.ops: Operator -> *Operations (concrete type) - NewOperatorRegistry: Operator -> *Operations - Compile-time checks: one per sub-interface (7 checks) - Sub-interfaces preserved for documentation/organization --- internal/rpn/operations.go | 14 +++++++---- internal/rpn/operations_interfaces.go | 44 +++++++---------------------------- internal/rpn/operator_registry.go | 2 +- internal/rpn/rpn_state.go | 2 +- 4 files changed, 20 insertions(+), 42 deletions(-) diff --git a/internal/rpn/operations.go b/internal/rpn/operations.go index 99b8e6b..3dfc7c2 100644 --- a/internal/rpn/operations.go +++ b/internal/rpn/operations.go @@ -17,10 +17,16 @@ type Operations struct { mu sync.RWMutex } -// Ensure Operations implements Operator at compile time. -// This is an explicit interface satisfaction check that will fail to compile -// if Operations doesn't implement all methods required by the Operator interface. -var _ Operator = (*Operations)(nil) +// Ensure Operations implements all operator sub-interfaces at compile time. +var ( + _ ArithmeticOperator = (*Operations)(nil) + _ BooleanOperator = (*Operations)(nil) + _ HyperOperator = (*Operations)(nil) + _ StackOperator = (*Operations)(nil) + _ VariableOperator = (*Operations)(nil) + _ ConstantOperator = (*Operations)(nil) + _ PowerIntOperator = (*Operations)(nil) +) // NewOperations creates a new Operations instance with the given variable store. // Creates a new ConstantsProvider internally; use SetConstants to replace it. diff --git a/internal/rpn/operations_interfaces.go b/internal/rpn/operations_interfaces.go index b254a26..5acce62 100644 --- a/internal/rpn/operations_interfaces.go +++ b/internal/rpn/operations_interfaces.go @@ -68,40 +68,12 @@ type PowerIntOperator interface { FastPower(stack *Stack) error } -// Operator is the combined interface for all operator implementations. -// This allows RPN to depend on an abstraction instead of the concrete Operations type. +// Operator implementations are split across focused sub-interfaces +// (ArithmeticOperator, BooleanOperator, HyperOperator, StackOperator, +// VariableOperator, ConstantOperator, PowerIntOperator) for clarity. +// The combined Operator interface was removed — RPN is the sole client +// and Operations is the sole implementor, so the interface added +// indirection without practical benefit (ISP). // -// Design note: Operator intentionally mixes behavioral methods (arithmetic, stack, -// boolean ops) with configuration methods (SetMode, SetPrefixMode, GetPrefixMode), -// metric command handlers, and custom metric commands. Per ISP this could be split, -// but RPN is the sole client and splitting would add indirection without practical -// benefit. The concrete *Operations type satisfies this interface exclusively. -type Operator interface { - ArithmeticOperator - BooleanOperator - HyperOperator - StackOperator - VariableOperator - ConstantOperator - PowerIntOperator - // SetMode sets the calculation mode (e.g., FloatMode, RationalMode). - SetMode(CalculationMode) - // GetMode returns the current calculation mode. - GetMode() CalculationMode - // SetPrefixMode sets the prefix mode for data size calculations - SetPrefixMode(PrefixMode) - // GetPrefixMode returns the current prefix mode - GetPrefixMode() PrefixMode - // Metric command handlers - MetricShow(stack *Stack) (string, error) - MetricList(stack *Stack) (string, error) - MetricCategory(stack *Stack, categoryName string) (string, error) - MetricCompatible(stack *Stack) (string, error) - // Custom metric commands - CustomShow(stack *Stack, name string) (string, error) - CustomList(stack *Stack) (string, error) - CustomDefine(name string, factor float64, category string) error - CustomUndefine(name string) error - // MetricRegistry returns the metric registry used by this Operations instance. - MetricRegistry() *MetricRegistry -} +// Each sub-interface is satisfied by *Operations, verified by compile-time +// checks in operations.go. diff --git a/internal/rpn/operator_registry.go b/internal/rpn/operator_registry.go index 2e5d702..8fca209 100644 --- a/internal/rpn/operator_registry.go +++ b/internal/rpn/operator_registry.go @@ -17,7 +17,7 @@ type OperatorRegistry struct { } // NewOperatorRegistry creates a new operator registry and registers all operators. -func NewOperatorRegistry(op Operator) *OperatorRegistry { +func NewOperatorRegistry(op *Operations) *OperatorRegistry { registry := &OperatorRegistry{ standardOperators: make(map[string]OperatorHandler), hyperOperators: make(map[string]OperatorHandler), diff --git a/internal/rpn/rpn_state.go b/internal/rpn/rpn_state.go index 2721cd1..458973f 100644 --- a/internal/rpn/rpn_state.go +++ b/internal/rpn/rpn_state.go @@ -14,7 +14,7 @@ type RPN struct { mu sync.RWMutex vars VariableStore consts ConstantsProvider - ops Operator + ops *Operations opRegistry *OperatorRegistry assignHandler *assignmentHandler maxStack int -- cgit v1.2.3