Concerns about multiple model queries and array management

Rails
1

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:

2

Not sure if anyone is reading any of this, but figured I would update it
to show what things I’m doing to try and resolve my own code and clean
it up.

First, I took a step back and realized I was doing too much in my rake
file, and not enough in the model. So, all I have in my rake file for
this particular ratings bit is:

update_tsrs = TsrsRating.new
offensive_opponent_ratings = []
offensive_opponent_ratings = update_tsrs.calculate_ratings(Schedule,
TsosOffense)
offensive_opponent_ratings.each_with_index do |row,i|
puts " Team ID of #{i+1} has opponent ratings of #{row}"
end

that’s it…

The model then does all of the real work and returns the end result back
to the rake file to be held in queue until all the tallies are
completed.

I also realized that I wasn’t keeping things DRY. I should have created
a class for the entire routine - so I did. I created a class that holds
the entire routine and made it so that I can call it with the remainder
of my ratings subtasks…

This should help.

I also realized that it doesn’t matter what my array and variable names
are so long as they are easy to read. Before they weren’t. I was even
getting lost myself.

So, I made them simpler to read using names like:

opponent_strength
opponent_array
opponent_ids
find_opponent_by_id
etc.

So, now I can at least read my own code which is hopefully a good thing.

I still have the same times loops and array loops in my code as I listed
in pastie. I will have to work at cleaning that up as well.

Thanks.

3

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…

4

On Sun, Jul 19, 2009 at 10:01 PM,
Älphä Blüë[email protected] wrote:

http://pastie.org/551660

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.

5

Thanks guys - I’m going to look at cleaning this up a bit. The 120
stems from the fact that there will always be exactly 120 teams (well
until the year 2012-2013) when South Alabama joins Div. 1.

My initial problem started when I didn’t understand how to organize two
arrays into a hash of hashes and sort through them. The example given
used 120.times and I kind of inherited that mechanic going forward. In
hindsight, I knew it felt wrong but I just didn’t get to the cleanup.

My focus problem with ruby iteration is understanding .each and
.each_with_index when handling array of arrays.

When querying data from a model it’s stored into an array. When using
each you iterate through each row and the value passed to the block is
used for iterating through that row.

example = model.find(:all)
example.each do |row|
puts “id = #{row.id} and name = #{row.name}”
end

Correct? Seems simple enough… however,…

What if I have an array of arrays:

First glance I would think I would use each_with_index to handle an
array of arrays. But, I believe I’ve been having trouble using that
because by using times in the way that I have been some arrays are
populated and starting with 1 versus 0.

I’ll try cleaning up the code and post a cleanup version for you to look
at to see if I’m progressing correctly.

Thanks again.

6

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.

7

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.

On Mon, Jul 20, 2009 at 9:07 AM, Älphä
Blüë[email protected] wrote:

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

8

Ä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,

Marnen Laibow-Koser
http://www.marnen.org
[email protected]

9

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…

10

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.

11

class TsosSpecialTeams < ActiveRecord::Base
belongs_to :team
end

… is actually supposed to read:

class TsosSteams
belongs_to :team
end

12

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

On Mon, Jul 20, 2009 at 11:01 AM, Älphä Blüë <
[email protected]> wrote:

has_many :opponents, :through => :schedules
class Schedule < ActiveRecord::Base
belongs_to :team

–
Posted via http://www.ruby-forum.com/.

–
Rick R

13

Rick,

I was also looking at your example - this is what I would like to do:

schedules = Schedule.find(:all, :joins => [:team, :opponent,
:tsos_offense], :order => :team_id)
schedules.each do |schedule|
val = 0
schedule.opponents.each do |opponent|
if opponent.id == nil
val += 0.2511
else
val += schedule.tsos_offenses.totals
puts “Team name = #{schedule.team.name} and opp_id =
#{schedule.opponent.id} and sum of val now = #{val} pulled from original
value of #{schedule.tsos_offense.totals}.”
end
puts “Team”
end
end

When it comes down to it, each team, opponent has a ratings assigned to
team_id in tsos_offense. So, couldn’t I just somehow join the ratings
tables with schedules and setup all the information side by side and
iterate through it?

Example diagram:

Team has …

id
name

Schedule has …

team_id
opponent_id

Tsos Offense has …

team_id
totals

======================================
Name | team_id | opponent_id | totals

What would I have to do in tsos_offense to make the association work and
is my loop above correct once the association is in place?

14

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.