Description
The bunko_collection macro has a confusing name collision where a class method includes a concern, then immediately calls an instance method with the exact same name. This violates the principle of least surprise and makes the code difficult to follow.
Current code
lib/bunko/controllers/acts_as.rb:8-14:
class_methods do
def bunko_collection(collection_name, **options)
include Bunko::Controllers::Collection
bunko_collection(collection_name, **options) # Calls instance method!
end
end
Problems this causes
- Extremely confusing - same name for class and instance methods
- Hard to trace which method is being called
- Violates principle of least surprise
- Makes debugging difficult
- Could cause issues with metaprogramming or introspection
- New contributors will struggle to understand the flow
Pros of refactoring
- Clear separation between macro invocation and configuration
- Easier to understand code flow
- Better for debugging and stack traces
- Follows Rails naming conventions (e.g.,
has_many vs internal setup methods)
- More maintainable
Cons of refactoring
- Small internal change (not user-facing)
- Need to choose a good alternative name
Recommended approach
Rename the instance method in the Collection concern to something like configure_bunko_collection or setup_bunko_collection:
# In acts_as.rb
class_methods do
def bunko_collection(collection_name, **options)
include Bunko::Controllers::Collection
setup_bunko_collection(collection_name, **options)
end
end
# In collection.rb
class_methods do
def setup_bunko_collection(collection_name, **options)
# existing implementation
end
end
This makes it clear that the user-facing API is bunko_collection, while the internal setup is separate.
References
lib/bunko/controllers/acts_as.rb:8-14
lib/bunko/controllers/collection.rb:14-36
Description
The
bunko_collectionmacro has a confusing name collision where a class method includes a concern, then immediately calls an instance method with the exact same name. This violates the principle of least surprise and makes the code difficult to follow.Current code
lib/bunko/controllers/acts_as.rb:8-14:Problems this causes
Pros of refactoring
has_manyvs internal setup methods)Cons of refactoring
Recommended approach
Rename the instance method in the Collection concern to something like
configure_bunko_collectionorsetup_bunko_collection:This makes it clear that the user-facing API is
bunko_collection, while the internal setup is separate.References
lib/bunko/controllers/acts_as.rb:8-14lib/bunko/controllers/collection.rb:14-36