Refactor Unit class for improved code quality and maintainability

[olbrich]

This pull request refactors the RubyUnits::Unit class and related components to improve code quality, maintainability, and adherence to Ruby best practices. The changes address several code smells identified by static analysis tools while maintaining backward compatibility.

Key Changes

Configuration and Static Analysis

• Added .reek.yml configuration file to document and manage intentional design patterns:
  • Disabled InstanceVariableAssumption detector (justified by mixed eager/lazy initialization pattern)
  • Disabled RepeatedConditional warnings for legitimate repeated checks across different contexts

Encapsulation Improvements

• Changed data attribute in RubyUnits::Cache from attr_accessor to attr_reader for better encapsulation
• Converted multiple class-level attributes in RubyUnits::Unit to attr_reader with private attr_writer methods

Code Refactoring and Cleanup

• Simplified return_scalar_or_unit method: Removed redundant parameter - method was always called with identical values for both arguments
• Extracted complex methods into smaller, focused helper methods:
  • register_prefix_definition, register_unit_definition, create_unit_value, register_aliases for unit definition registration
  • validate_power_operation, special_power_case, calculate_power_result for power operations
  • apply_signature_items for signature vector calculations
  • validate_time_point, ensure_compatible_with for validation logic
  • return_scalar_or_raise for type conversion methods
  • validate_unit_string_format, invalid_unit for parsing validation
• Replaced nil comparisons with NilClass type checks for clarity and Reek compliance
• Improved method documentation throughout
• Restructured conditionals to reduce nesting and improve readability
• Used named regex captures for clearer intent in parsing logic
• Consolidated repetitive code patterns into reusable helper methods

Performance Optimizations

• Reduced duplicate method calls by caching values
• Improved efficiency in unit signature vector calculations
• Streamlined unit conversion logic

All existing tests pass, ensuring backward compatibility is maintained.

[Copilot]

Pull request overview

This pull request refactors the RubyUnits::Unit class to improve code quality and maintainability while maintaining backward compatibility. The changes include encapsulation improvements, method extractions, and code simplifications based on static analysis recommendations.

Key changes:

• Converted class and instance level attr_accessor to attr_reader with private attr_writer methods for better encapsulation
• Extracted complex methods into smaller, focused helper methods for improved readability and maintainability
• Added .reek.yml configuration to document intentional design patterns and suppress false positives

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
.reek.yml	New configuration file to document and manage intentional design patterns and suppress false positive warnings
lib/ruby_units/cache.rb	Changed data attribute from attr_accessor to attr_reader for improved encapsulation
lib/ruby_units/unit.rb	Major refactoring including encapsulation improvements, method extractions, nil comparison updates, lambda-based refactoring of to_base, improved error handling, and addition of numerous helper methods for validation and formatting

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The change from default parameter values to nil-checking within the method body may alter behavior when false is explicitly passed as an argument. If precision: false or format: false is passed, the original code would use false, but the new code will treat false as falsy and use the configuration defaults instead. This could be a subtle breaking change in behavior.

Suggested change

	precision ||= configuration.default_precision
	format ||= configuration.format
	precision = configuration.default_precision if precision.nil?
	format = configuration.format if format.nil?

[olbrich]

@copilot This is OK. false is not a valid value for either of these configurations.

[Copilot]

Moving the zero check before the case statement changes the order of error checking. If a Unit is zero, the original code would first check if it's a temperature (line 973) before raising ZeroDivisionError. Now, ZeroDivisionError is raised before temperature validation, changing the error behavior for edge cases like dividing by a zero-valued temperature unit.

[Copilot]

The change from @scalar.nil? to !@scalar changes the semantic meaning. The original code explicitly checked for nil, but the new code treats any falsy value (including false, 0, empty string) as truthy for the negation. This could lead to different behavior if @scalar is 0 or false, though in this context @scalar is likely always either nil, a string, or a Numeric. However, it's safer and clearer to use .nil? for nil checks.

Suggested change

	@scalar = @scalar.to_f unless !@scalar || @scalar.empty?
	@scalar = @scalar.to_f unless @scalar.nil? || @scalar.empty?

[Copilot]

The change from bottom_scalar.nil? to !bottom_scalar changes the semantic meaning. The original code explicitly checked for nil, but the new code treats any falsy value (including false, 0) as truthy for the negation. While in this parsing context bottom_scalar is likely always either nil or a string, using explicit .nil? checks is clearer and more maintainable.

Suggested change

	bottom_scalar = 1 if !bottom_scalar || bottom_scalar.empty?
	bottom_scalar = 1 if bottom_scalar.nil? || bottom_scalar.empty?

[Copilot]

The change from unless @signature.nil? to if @signature changes behavior when @signature equals 0. A signature of 0 is a valid value (for unitless quantities or when the signature vector sums to zero), but the new code will treat 0 as falsy and recalculate the signature unnecessarily. This could impact performance and potentially cause issues with caching.

Suggested change

	return @signature if @signature
	return @signature unless @signature.nil?

[Copilot]

@olbrich I've opened a new pull request, #383, to work on those changes. Once the pull request is ready, I'll request review from you.