Refactor: Simplify Setting Methods and Remove Dynamic Method Generation

[ericproulx]

Refactor: Simplify Setting Methods and Remove Dynamic Method Generation

Overview

This PR refactors the Grape DSL settings implementation to improve code clarity, maintainability, and performance by replacing dynamic method generation with explicit method definitions and simplifying the get_or_set method.

Changes Made

1. Simplified get_or_set Method

• Before: Complex method that took a type symbol and used __send__ to access setting objects
• After: Simple method that works directly with setting objects, eliminating the need for dynamic dispatch

# Before
def get_or_set(type, key, value)
  setting = inheritable_setting.__send__(type)
  if value.nil?
    setting[key]
  else
    setting[key] = value
  end
end

# After
def get_or_set(setting, key, value)
  return setting[key] if value.nil?
  setting[key] = value
end

2. Replaced Dynamic Method Generation with Explicit Methods

• Before: Used define_method to dynamically create setting methods
• After: All setting methods are now explicitly defined, making the code more readable and easier to debug

# Before
%i[namespace_inheritable namespace_stackable].each do |method_name|
  define_method method_name do |key, value = nil|
    get_or_set method_name, key, value
  end
end

# After
def namespace_inheritable(key, value = nil)
  get_or_set(inheritable_setting.namespace_inheritable, key, value)
end

def namespace_stackable(key, value = nil)
  get_or_set(inheritable_setting.namespace_stackable, key, value)
end

3. Optimized Logger Implementation

• Before: Used global_setting method which added an extra layer of indirection
• After: Directly accesses global settings for better performance

# Before
def logger(logger = nil)
  if logger
    global_setting(:logger, logger)
  else
    global_setting(:logger) || global_setting(:logger, ::Logger.new($stdout))
  end
end

# After
def logger(logger = nil)
  global_settings = inheritable_setting.global
  if logger
    global_settings[:logger] = logger
  else
    global_settings[:logger] || global_settings[:logger] = ::Logger.new($stdout)
  end
end

4. Improved Test Coverage

• Removed tests that only verified delegation behavior
• Added comprehensive tests that verify actual functionality
• Updated logger tests to use the proper DSL modules

Benefits

1. Improved Readability: Explicit method definitions are easier to understand than dynamic generation
2. Better Performance: Eliminated unnecessary method calls and dynamic dispatch
3. Easier Debugging: Stack traces are clearer with explicit methods
4. Reduced Complexity: Removed 45 lines of code while maintaining the same functionality
5. Better Test Coverage: Tests now focus on actual behavior rather than implementation details

Files Changed

• lib/grape/dsl/settings.rb - Refactored setting methods and get_or_set implementation
• lib/grape/dsl/logger.rb - Optimized logger to directly access global settings
• spec/grape/dsl/settings_spec.rb - Updated tests to focus on behavior
• spec/grape/dsl/logger_spec.rb - Simplified test setup

Testing

All existing tests pass, and the refactoring maintains backward compatibility. The changes are purely internal improvements to the DSL implementation.

Breaking Changes

None. This is a backward-compatible refactoring that maintains the same public API.