Today I was doing a read-through of existing code to prepare for an upcoming feature, and I came across this abomination.
# controller code
# GET /project_team_tasks
def show
@teams = project.teams
TeamTask.define_teams_quantity_distributions_attr_readers(@teams)
end
# model code
def self.define_teams_quantity_distributions_attr_readers(teams)
teams.each do |team|
Budget.component_groups.each do |component_group|
define_method("team_#{team.id}_assembled_elements_quantity_#{component_group}") do
teams_quantity_distributions["team_#{team.id}_assembled_elements_quantity_#{component_group}"]
end
end
end
end
# then in view
team.tasks.send("team_#{team.id}_assembled_elements_quantity_1")
As far as I understand, calling the class method defines an instance method (a getter) for every team*component_group pair. Apparently the underlying data are stored in TeamTask instance teams_quantity_distributions jsonb field.
This code is pernicious because it's a source of memory leak - given a sufficiently large number of teams and component groups we can end up having dynamically defined thousands of instance methods on TeamTask, and this will occur only as action is called for different projects. Plus, it's wholly unnecessary, why not just access the teams_quantity_distributions field in a regular manner (one still needs to compose the method name same as a has key), or use method_missing metaprogramming to have just one method definition?!
What hairy code have you come across lately?
Top comments (0)