← Back to Upcase

Where should this query go? Possible code smell?

(Cyle Hunter) #1

I’ve reached a fairly tricky design hurdle for a semi complex Rails application. It’s a tournament matchmaking system with an STI interface for match data based on whether or not the associated Tournament is round-robin or elimination format.

For both formats, matches will have a home_id, away_id, winner_id, and round. The home and away ids correspond to a registration id, and the winner id corresponds to either the home or away id (which in turn represent a registration id). In the case of elimination tournaments, a registrant’s “elmination” is represented by the lack of a presence in any given matches winner_id field.

Here’s what my models are looking like:

class Registration < ActiveRecord::Base
  belongs_to :tournament
  has_many :matches, :class_name => "Match", :foreign_key => "home_id"
  has_many :matches_contending, :class_name => "Match", :foreign_key => "away_id"
  has_many :matches_won, :class_name => "Match", :foreign_key => "winner_id"

class Tournament < ActiveRecord::Base
  has_many :registrations
  has_many :matches

class Match < ActiveRecord::Base
  belongs_to :tournament
  belongs_to :home, :class_name => "Registration"
  belongs_to :away, :class_name => "Registration"
  belongs_to :winner, :class_name => "Registration"

class Elimination < Match; end

class RoundRobin < Match; end

The problem I’m facing is that the user requires a list of list of registrants to select from for the home_id and away_id fields for the elimination matches, as they have to be inputted manually for each match within each round. I don’t trust the user to know which selections are valid (they might accidentlly select a player that has already been eliminated in a later round), instead I’ll supply them with data which I know to be valid. This is a non-issue for the round robin matches as the system automatically assigns these fields when it generates the matches required for the Tournament, the user simply has to assigns the winner.

Currently I have a function in my competitions model which suplies me with a list of valid registrants that can be entered as participants in the currently active round. Here it is:

def selectable_registrants_for_round(round)
  eliminated = (registrations.joins(:matches).where("registrations.id != winner_id") +
  registrations.joins(:matches_contending).where("registrations.id != winner_id"))
  round_matches = eliminations.where(round: round)
  already_assigned_ids_for_round = (round_matches.map(&:home_id) +
  if eliminated.any?
    return registrations.where("id not in (?)",eliminated.map(&:id) + 
    return registrations

As you can see there’s a bit of awkward array concatenation going on above, that’s just because home, away, and winners all correspond to the same registration model. What I’m more concerned with is:

  1. It’s in my Tournament model, despite only being relevant for elimination-format tournaments.
  2. With all the calls that are going on between the registration and match models, and the fact that it returns an array of registration records, it isn’t obvious to me which model this method should belong to.

The code in it’s current state works, but I don’t feel comfortable about this method or where it’s situated.

(Cyle Hunter) #2

I ended up moving it to my Elimination model. At the end of the day, this query is only going to be used for Elimination data. Neither Competitions and Registrations require this query in order to carry out their respective roles.