Concerns about multiple model queries and array management

Wow, what a week! I've gotten a lot accomplished with my site but
brain-fried has set into my head..

I'm reaching a point where I'm having to manipulate arrays to gain
access to ids that I will use in model.find methods.

Here's a quick and probably dirty example of what works. I'm only
showing it here because I know it's gotta be dirty and ugly and there's
probably a better way of doing it, but I can't seem to figure it out.

My best guess is I should be using each and each_with_index but anyhoo..

Providing the methods as is, with a brief idea of what data is
returned..

open_schedule = Schedule.new
opp_scheduled_ids = []
120.times do |i|
  opp_scheduled_ids[i+1] = open_schedule.find_opponents(i+1)
  puts "Team ID = #{i+1} and Scheduled IDs are
#{opp_scheduled_ids[i+1].join(',')} and Array Size =
#{opp_scheduled_ids[i+1].size}"
end

This looks into my schedule model/table, finds 120 teams and any
opponent they've scheduled within date > datetime value and date <
datetime value. It then returns the ids for those opponents. Data looks
like this:

Team ID = 1 and Scheduled IDs are
25,27,65,120,6,62,119,92,109,100,121,111,9 and Array Size = 13
Team ID = 2 and Scheduled IDs are
54,85,51,29,110,116,106,22,19,113,48,49 and Array Size = 12
Team ID = 3 and Scheduled IDs are
100,58,52,91,27,68,74,111,115,32,118,54 and Array Size = 12
Team ID = 4 and Scheduled IDs are
121,99,78,89,105,33,35,108,73,92,103,56 and Array Size = 12
etc...

So, I'm able to hold the Team ID for the team I'm checking, and all of
their opponent_ids that they've scheduled based off whatever date
parameters I've selected. opponent_id is a foreign key that really is
the same as team_id but in an opponent's column.

So far so good. Again, might be sloppy..

Now I want to pull out each of the opponent_ids that are stored in each
team's array..

120.times do |i|
  opp_scheduled_ids[i+1].size.times do |x|
    puts "Team ID = #{i+1} and opp_id = #{opp_scheduled_ids[i+1][x]}"
  end
end

This gives me the correct information that I'm looking for:

Team ID = 1 and opp_id = 25
Team ID = 1 and opp_id = 27
Team ID = 1 and opp_id = 65
Team ID = 1 and opp_id = 120
Team ID = 1 and opp_id = 6
Team ID = 1 and opp_id = 62
Team ID = 1 and opp_id = 119
Team ID = 1 and opp_id = 92
Team ID = 1 and opp_id = 109
Team ID = 1 and opp_id = 100
Team ID = 1 and opp_id = 121
Team ID = 1 and opp_id = 111
Team ID = 1 and opp_id = 9
Team ID = 2 and opp_id = etc.....
etc....

So, what I plan on doing is issuing a find method for each opponent id
to find out a rating in a specific table and sum them all up. The find
method will go in this laste method that I showed you that only houses a
puts statement so far..

But, as you can see that method is very ugly...

What can I do to clean it up so that it still does the same thing but
with cleaner code or efficiency? Again, my purpose is to learn better
array management with rails, especially since I will be doing a find
command.

In addition, my concerns are that I could probably just do "one" find
command for all of the rating values in a specific table and then search
within those results, matching up the opponent_id to the team_id and the
value..

Example:

I want to search for the totals for team_id in offense ratings table.
Do a find(:all) to pull all totals in offense ratings table.
Find the totals for opp_id = # where team_id = #...

This would be better, IMO, because I'm not pulling 120 separate queries
but one large query and searching in the cached results... correct?

Any advice will be greatly "appreciated" regarding my concerns..

Hey Marnen.. I remembered :slight_smile:

http://pastie.org/551660

I added a pastie of the current rake file task (yep still working on a
procedural piece for ratings automation), along with the model (in full
for schedules) etc.

A brief idea of how it works and what I don't like about it...

In order to obtain a ratings schedule of strength for each team, I need
to find all of the ratings for each team they play...

This is why scheduling is so important to my project..

So, I'm simply finding all opponents for each team, then finding all the
ratings for each opponent, and at the moment just doing a simple sum.
It will get a lot more difficult as I place it into my Standard
deviation variance model but I can do it. I just don't like the "way"
I'm doing it..

Look in the pastie code and there's a small notes section right after
the rake file. There's one bit of code which looks enormously ugly and
inefficient but it matches and works 100%! The cliche is if it's not
broke don't fix it. But, even though it's not broke, it just doesn't
feel rails like to me or ruby like to me. It feels hackish.

This code here for instance:

val += opp_off_tsos[opp_scheduled_ids[i+1][x]-1]

.. what!!?

Well break it down..

opp_off_tsos (translates to array for the rating itself)
opp_scheduled_ids[i+1][x] (translates to the opponent_id)
.... i+1 is because the 120.times starts with 0 and I need to start with
1..
.... x is a true 0 start because I used .each..
Why -1 at the end? Because the opp_off_tsos array was created with
120.times so it starts with 0 and I need to subtract one from the id to
get it to match..

Again, it all works but as you can see, inefficient and ugly..

I haven't investigated all of this, and I'm coming from a Java/Groovy
background and new to Rails, but when I see 120.times to me it throws
up a red flag. Can't you make this more OO? For example I would think
you could say Find all teams and for each team there would be a
collection of "opponents." (I remember a while back we had a
discussion on the model for this project but forgot how it was
resolved.)

I would think the query itself would take care of a lot of this to
avoid certain array constructs. I'd think lines 10-13 would just
become teams = Teams.find(:all) (where in each time you'd have
collection opponents )

Then later, something like:

teams.each do |team|
val = 0
team.opponents.each do |opponent|
if opponent.schedule_id == 121
val += 0.2511
     else
         val += opp_off_tsos[opponent.id]
     end
  end
end

Maybe I'm missing something (and I'm sure I am) but it sure seems
cleaner using nested collections than doing all that array stuff.

Älphä Blüë wrote:
[...]

Providing the methods as is, with a brief idea of what data is
returned..

I don't have time to review all these in depth, but:

open_schedule = Schedule.new
opp_scheduled_ids = []
120.times do |i|

Why are you hard-coding the value 120?

  opp_scheduled_ids[i+1] = open_schedule.find_opponents(i+1)
  puts "Team ID = #{i+1} and Scheduled IDs are
#{opp_scheduled_ids[i+1].join(',')} and Array Size =
#{opp_scheduled_ids[i+1].size}"
end

Wow. Notice first of all that you're using i+1 a lot in each iteration.
If that's even necessary -- and I'm not sure why it would be -- then you
should put into a variable.

In any case, your times loop is probably unnecessary; each_with_index
would be better. Or if find_opponents could take a Team object as
argument rather than an I'd value, you could use collect to slim this
down even more.

This looks into my schedule model/table, finds 120 teams and any
opponent they've scheduled within date > datetime value and date <
datetime value. It then returns the ids for those opponents.

You're reinventing DB queries in the application layer. This should
probably be done in the DB.
[...]

Now I want to pull out each of the opponent_ids that are stored in each
team's array..

120.times do |i|
  opp_scheduled_ids[i+1].size.times do |x|

size.times? Why not each_with_index?

    puts "Team ID = #{i+1} and opp_id = #{opp_scheduled_ids[i+1][x]}"
  end
end

Why the i+1 all over the place? If you really need to start from 1, use
(1..120).each. But it would probably be cleaner to use AR and/or a DB
query for this.
[...]

What can I do to clean it up so that it still does the same thing but
with cleaner code or efficiency?

Stop trying to write Rails like spaghetti PHP! Learn to get the most
out of Ruby's Array methods (there are *lots*) and ActiveRecord. Get
used to doing big queries in the DB.

[...]

In addition, my concerns are that I could probably just do "one" find
command for all of the rating values in a specific table and then search
within those results, matching up the opponent_id to the team_id and the
value..

Example:

I want to search for the totals for team_id in offense ratings table.
Do a find(:all) to pull all totals in offense ratings table.
Find the totals for opp_id = # where team_id = #...

This would be better, IMO, because I'm not pulling 120 separate queries
but one large query and searching in the cached results... correct?

Probably. Aggregate functions will make your life much easier here.

Any advice will be greatly "appreciated" regarding my concerns..

Hey Marnen.. I remembered :slight_smile:

I see! LOL.

Best,

Okay I just want to make sure I understand this better.

schedules = Schedule.find(:all)
schedules.each do |schedule|
  puts "Team Id = #{schedule.team_id}"
end

That shows me all the team_ids listed in the schedules table. Now what
if I want to iterate through those team_ids and collect all of their
opponent ids together?

team Id 1 -> Opponent Id 3
team Id 1 -> Opponent Id 94
etc.

Each team has more than one opponent scheduled. The id fields are
team_id and opponent_id.

I tried:

schedules = Schedule.find(:all)
schedules.each do |schedule|
  schedule.team_id.each do |team|
    puts "Team Id = #{schedule.team_id} and Opponent Id =
#{team.opponent_id}"
  end
end

But that will give me no method each found for 6:fixnum because I'm
trying each on a team_id number..

How do I further iterate through each row returned to gather up the
data? I want to know what opponent_ids each team_id has.

Why don't you just set the model up where you find "Teams" and each
Team has a collection of Team objects called 'opponents'? It would
seem to be a lot easier to me this way.

That shows me all the team_ids listed in the schedules table. Now what
if I want to iterate through those team_ids and collect all of their
opponent ids together?

This part seems pretty messed up. I think what Marnen and I are trying
to say is why not aggregate this more? Why are so determined to work
off of "ids" ? In my opinion you should be going through "Team"
objects and not ids. And then for any given Team you'd have a nested
collection of "opponents" ActiveRecord I'm sure could bring all this
back to you in a more OO way. For example, what was wrong with my
pseduo code idea:

teams.each do |team|
   val = 0
   team.opponents.each do |opponent|
      if opponent.schedule_id == 121
         val += 0.2511
    else
        val += opp_off_tsos[opponent.id]
    end
end
end

Hi Rick,

I want to do it exactly the way you are saying but I think I'm just a
bit confused here.

Let me show you what I'm working with now:

schedules = Schedule.find(:all, :joins => [:team, :opponent], :order =>
:team_id)
schedules.each do |schedule|
  puts "Name = #{schedule.team.name} and ID = #{schedule.team.id} and
Opponent = #{schedule.opponent.id}"
end

I need to work out of schedules because there will be say 1,200 rows of
data. The teams table only contains 120 rows (for each team).

The associations I created were setup as:

class Schedule < ActiveRecord::Base
  belongs_to :team
  belongs_to :opponent, :class_name => "Team"
end

class Team < ActiveRecord::Base
  has_many :schedules
  has_many :inheritance_templates
  has_many :opponents, :through => :schedules
  has_many :tsos_offenses
  has_many :tsos_defenses
  has_many :tsos_steams
  has_many :tsos_turnover_margins
  has_many :tsrs_ratings
end

The above method does join both pieces together with teams. I'm just
unsure of how to iterate through them.

I tried what you said using opponent and even team as the next each
iteration but got no method each for those two..

Let me take a step back here and look over my associations:

Are they setup incorrectly?

I want team to be my core object always..

class Team < ActiveRecord::Base
  has_many :schedules
  has_many :inheritance_templates
  has_many :opponents, :through => :schedules
  has_many :tsos_offenses
  has_many :tsos_defenses
  has_many :tsos_steams
  has_many :tsos_turnover_margins
  has_many :tsrs_ratings
end

I want schedule to be a part of teams and also to be a part of opponent
(self-referential) through Team..

class Schedule < ActiveRecord::Base
  belongs_to :team
  belongs_to :opponent, :class_name => "Team"
end

These two pieces alone should give me associations between Team >
Schedule > Opponent.

My ratings tables all belong to team:

class TsosOffense < ActiveRecord::Base
  belongs_to :team
end
class TsosDefense < ActiveRecord::Base
  belongs_to :team
end
class TsosSpecialTeams < ActiveRecord::Base
  belongs_to :team
end
class TsosTurnoverMargin < ActiveRecord::Base
  belongs_to :team
end

Everyone and everything I read, including you both (Rick and Marnen) say
that large queries are much better than smaller queries.

What if I want to query everything from all models listed here, assign
them to a team object so that I can reference them by the team object.
How would I do that?

If I have the answer to this one question, I think I can fix all of the
current issues I'm experiencing.

class TsosSpecialTeams < ActiveRecord::Base
  belongs_to :team
end

.. is actually supposed to read:

class TsosSteams
  belongs_to :team
end

I’ve been busy the past two days. How have things worked out with all this?

The associations were fine. I did find a way to implement my ratings
system and it works solidly. I built everything in the models and now I
only use rake files for task/procedure initialization. The models all
house the meat of my routines/methods for calculations.