DEV Community

Augusts Bautra
Augusts Bautra

Posted on

Share your cancer code!

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")
Enter fullscreen mode Exit fullscreen mode

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)