Concerns about multiple model queries and array management

rubyonrails-talk
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]}"   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

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]-1]

.. what!!?

Well break it down..

opp_off_tsos (translates to array for the rating itself) opp_scheduled_ids[i+1] (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..

3

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.

4

Ä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]}"   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,

5

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.

6

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

7

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..

8

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.

9

class TsosSpecialTeams < ActiveRecord::Base   belongs_to :team end

.. is actually supposed to read:

class TsosSteams   belongs_to :team end

10

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

11

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.