frc-frecon / frecon

An API for building scouting apps for FRC competitions
MIT License
3 stars 0 forks source link

Better Schema #60

Closed rye closed 9 years ago

rye commented 9 years ago

This is a massive change in the underlying design of this application, but I think it works well.

Basically, Robots now (semantically) belong to teams, Participations (semantically) belong to robots, And Records (semantically) belong to participations.

I will update a new schema diagram as soon as I possibly can for comment and visualization.

EDIT: Here is a readiness checklist:

EDIT: Adding to the readiness checklist:

EDIT: (The above 3 were done in #61, :sparkles: :tada:)

rye commented 9 years ago

Here is the schema diagram:

new doc 2_1

rye commented 9 years ago

Should I make a relationship between Participation and Competition (n-to-1 respectively)?

rye commented 9 years ago

I think I will. That seems semantic.

rye commented 9 years ago

Here is the updated schema:

new doc_1

Sammidysam commented 9 years ago

Wait, why exactly was it that just removing the Robot model was not okay? Or do you find this just to be a better schema overall?

rye commented 9 years ago

I think this is better overall. It seems to make a lot more sense to me.

rye commented 9 years ago

As in, this is semantic.

rye commented 9 years ago

The main change here is that Robots have many Participations, and Records then belong to Participations instead, which makes sense, since you're taking notes on Robots as they Participate, not Robots from Competition to Competition. Also, the two models are now much more distinct.

rye commented 9 years ago

Feasibly, Competitions and Matches now have many Robots and Teams through Participations (though the actual logic for getting these has to be done manually because Mongoid doesn't support has_many :through associations, it seems), and both Teams and Robots can have many Records, Matches, and Competitions through Participations.

Sammidysam commented 9 years ago

This looks pretty solid to me. Do you happen to have an image of an old diagram for the old schema we made? I might look for one in my files. Just for a comparison.

rye commented 9 years ago

There was one in this comment on #56, to my recollection. Not sure that that was totally accurate, but I think it was.

rye commented 9 years ago

Though I think I might have been a bit confused regarding the previous schema. I will draw one up from the code that this branch came off of.

Sammidysam commented 9 years ago

Okay, sounds good. I couldn't find anything and was about to offer to do that myself, but go ahead.

rye commented 9 years ago

Okay, this was drawn up from the schema that I interpreted from the thing schema that I read on master. Seems really borked, now that I look at it.

img_20150629_203246

Sammidysam commented 9 years ago

Where does information about the robot get stored in this schema? I seem to remember information about robots changing a lot between competitions, and if you are tracking all of the competitions that a robot was in, it seems like you would want to record the differences in it at these competitions as well.

rye commented 9 years ago

In which schema, the new one?

I would think that Participations would hold the data about the Robot as it moved around between Competitions, especially since the Records belong_to :participations.

Sammidysam commented 9 years ago

Yes, the new one. Okay, I was just checking. So then a Robot is only present to show which competitions it participated in?

rye commented 9 years ago

You got it—the Robot model refers to the Team's design for a given season or game, and Participations are each of its "presences" at a competition or event (remember that model? I liked that model, but I like the Participation model better).

So, basically, yes, the Robot model serves to classify different years. But since teams don't completely re-do their robot from each Competition to each Competition, I think it would make sense to separate that out and make that distinction.

rye commented 9 years ago

Another advantage of this new schema is that we can then look at the Robot for a Team for a given year, and how it progresses across the year—or, if we wanted, we can look at all of the Robots for all of the years and how they each progressed.

Sammidysam commented 9 years ago

That sounds good. Does this merge also change how everything functions according to the schema change?

Sammidysam commented 9 years ago

Oh, nope, from the check list I doubt it does.

rye commented 9 years ago

Not yet. I was looking for a green light with the theory behind it—I'll put the helper methods and routes together as well now that it sounds like you like this.

Sammidysam commented 9 years ago

I think it should work fine. I assume most things on the outside would stay the same anyway.

rye commented 9 years ago

Yes, though WeBCa will need to change to match these changes once we merge them (specifically the request stuff). This is basically a major-version-level change, which is messy because we haven't released yet.

rye commented 9 years ago

I think that we should merge this once it's done, then build the FReCon.js library, then rewrite most of the backend behind WeBCa to make it work.

Sammidysam commented 9 years ago

This is why it is good we held off on releasing it I guess. :) I agree with this scheme for attack.

rye commented 9 years ago

Okay. Then we'll break the creed of semantic versioning and instead version this as another minor release (which is bad but not really because nobody cares at this point). WeBCa can point at FReCon v0.3 until we're done with the changes, then we can merge v0.4.

So I'll go ahead and bump the minor version on this branch.

Sammidysam commented 9 years ago

Actually, I thought semantic versioning said it doesn't really apply until 1.0. So then major changes are still minor changes because it is in development.

rye commented 9 years ago

Oh okay then, that makes me less uncomfortable with it. :grinning:

Then I'll just bump the version and then it'll be good.

rye commented 9 years ago

There now appear to be connections between all models.

rye commented 9 years ago

I checked most of the new methods via command-line on a test database with the new schema.

rye commented 9 years ago

@Sammidysam, If you have the time and could report back on the style of these selectors (that they are correctly-written), that would be awesome. They seem to work for me, but I'm not entirely sure that they are wholly correct.

Sammidysam commented 9 years ago

Looking at the code and diagram, they all looked good to me. I didn't run them, however. Did you check that there are not duplicate entries when you run them, and if there are that you run uniq? I assume you did, but it's worth checking.

rye commented 9 years ago

I do believe that I ran uniq when necessary. We can always come back and look at this later if something is giving us trouble. For now, I'll move on to the controllers and routes.

Sammidysam commented 9 years ago

:+1:

Sammidysam commented 9 years ago

Is the current query system not okay?

-Sam

Sent using CloudMagic [https://cloudmagic.com/k/d/mailapp?ct=pa&cv=6.3.45&pv=5.0.2]

On Tue, Jun 30, 2015 at 7:45 PM, Kristofer Rye < notifications@github.com [notifications@github.com] > wrote: Adding to the readiness checklist:

— Reply to this email directly or view it on GitHub [https://github.com/frc-frecon/frecon/pull/60#issuecomment-117373719] .[https://github.com/notifications/beacon/AC7z1unUzLqhr-TO38cC_tjBb8pbfpqiks5oYyGkgaJpZM4FOgLC.gif]

rye commented 9 years ago

The current query system has been effectively removed, so you can only get all of something; no filtering is available currently. I'd like to implement a smart filtering system, but maybe I'll refrain from doing that.

rye commented 9 years ago

Besides that, I've only got a little bit of refactoring to do with the existing routes logic. Then, some testing.

Sammidysam commented 9 years ago

The filtering system was just one line though. You could add it back.

-Sam

Sent using CloudMagic [https://cloudmagic.com/k/d/mailapp?ct=pa&cv=6.3.45&pv=5.0.2]

On Tue, Jun 30, 2015 at 10:05 PM, Kristofer Rye < notifications@github.com [notifications@github.com] > wrote: Besides that, I've only got a little bit of refactoring to do with the existing routes logic. Then, some testing.

— Reply to this email directly or view it on GitHub [https://github.com/frc-frecon/frecon/pull/60#issuecomment-117394632] .[https://github.com/notifications/beacon/AC7z1gs4twjbD122IsI_-MuHvNy29OfYks5oY0J8gaJpZM4FOgLC.gif]

rye commented 9 years ago

What line?

Sammidysam commented 9 years ago

Something along the lines of

Model = params ? Model.where(params) ? Model.all

-Sam

Sent using CloudMagic [https://cloudmagic.com/k/d/mailapp?ct=pa&cv=6.3.45&pv=5.0.2]

On Tue, Jun 30, 2015 at 10:44 PM, Kristofer Rye < notifications@github.com [notifications@github.com] > wrote: What line?

— Reply to this email directly or view it on GitHub [https://github.com/frc-frecon/frecon/pull/60#issuecomment-117413073] .[https://github.com/notifications/beacon/AC7z1pEldb5UGprDEo2RnqsNLXJtWjMxks5oY0uagaJpZM4FOgLC.gif]

rye commented 9 years ago

I can try and look at that, but I would like to look into a new system where query strings can be given via something like this:

GET /teams/2503/matches?competition_id=5591fd886461677062000001

--- or ---

GET /teams/2503/records?match+competition_id=5591fd886461677062000001

--- or ---

GET /teams/2503/records?match+competition+name="IRI"

--- or ---

GET /teams/2503/records?match+competition+id=5591fd886461677062000001

This would be fairly parsable, and fairly easy to implement, but requires a lot of thought about the parsing strategy.

Basically, each word that is not an expression (match is not an expression, but competition_id=5591fd886461677062000001 is an expression) is treated as a field; so the controller would first find the result for /teams/2503/records, then would split the params (which becomes a Hash like {"match+competition+id" => "5591fd886461677062000001"}) key into its parts: ['match', 'competition', 'id'], then maps each value in the /records thing to each of these things;

Iteratively, the resulting objects would look like each of these:

# This is the hypothetical flow for a query string of `match+competition+name="IRI"`

# [FReCon::Record, FReCon::Record, FReCon::Record, FReCon::Record]
=> [#<FReCon::Record _id: 559201ff6461677372000004, created_at: 2015-06-30 02:42:07 UTC, updated_at: 2015-06-30 02:42:07 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416673820 data=559201bc6461677372000002>, participation_id: <BSON::ObjectId:0x70194416673720 data=5591fdd96461677062000003>>,
 #<FReCon::Record _id: 5592020d6461677372000005, created_at: 2015-06-30 02:42:21 UTC, updated_at: 2015-06-30 02:42:21 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416672800 data=559201c66461677372000003>, participation_id: <BSON::ObjectId:0x70194416664460 data=5591fdd96461677062000003>>,
 #<FReCon::Record _id: 559202386461677372000006, created_at: 2015-06-30 02:43:04 UTC, updated_at: 2015-06-30 02:43:04 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416663600 data=559201c66461677372000003>, participation_id: <BSON::ObjectId:0x70194416663480 data=5591fdd96461677062000003>>,
 #<FReCon::Record _id: 559202406461677372000007, created_at: 2015-06-30 02:43:12 UTC, updated_at: 2015-06-30 02:43:12 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416662600 data=559201bc6461677372000002>, participation_id: <BSON::ObjectId:0x70194416662480 data=5591fdd96461677062000003>>]

# {FReCon::Record => FReCon::Match, FReCon::Record => FReCon::Match, FReCon::Record => FReCon::Match, FReCon::Record => FReCon::Match}
=> {#<FReCon::Record _id: 559201ff6461677372000004, created_at: 2015-06-30 02:42:07 UTC, updated_at: 2015-06-30 02:42:07 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416592780 data=559201bc6461677372000002>, participation_id: <BSON::ObjectId:0x70194418028800 data=5591fdd96461677062000003>>=>
  #<FReCon::Match _id: 559201bc6461677372000002, created_at: 2015-06-30 02:41:00 UTC, updated_at: 2015-06-30 02:41:00 UTC, number: "qm1", blue_score: 0, red_score: 0, competition_id: <BSON::ObjectId:0x70194416592300 data=5591fd886461677062000001>>,
 #<FReCon::Record _id: 5592020d6461677372000005, created_at: 2015-06-30 02:42:21 UTC, updated_at: 2015-06-30 02:42:21 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416563660 data=559201c66461677372000003>, participation_id: <BSON::ObjectId:0x70194418026960 data=5591fdd96461677062000003>>=>
  #<FReCon::Match _id: 559201c66461677372000003, created_at: 2015-06-30 02:41:10 UTC, updated_at: 2015-06-30 02:41:10 UTC, number: "qm2", blue_score: 0, red_score: 0, competition_id: <BSON::ObjectId:0x70194416563240 data=5591fd886461677062000001>>,
 #<FReCon::Record _id: 559202386461677372000006, created_at: 2015-06-30 02:43:04 UTC, updated_at: 2015-06-30 02:43:04 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416544720 data=559201c66461677372000003>, participation_id: <BSON::ObjectId:0x70194418025820 data=5591fdd96461677062000003>>=>
  #<FReCon::Match _id: 559201c66461677372000003, created_at: 2015-06-30 02:41:10 UTC, updated_at: 2015-06-30 02:41:10 UTC, number: "qm2", blue_score: 0, red_score: 0, competition_id: <BSON::ObjectId:0x70194416544320 data=5591fd886461677062000001>>,
 #<FReCon::Record _id: 559202406461677372000007, created_at: 2015-06-30 02:43:12 UTC, updated_at: 2015-06-30 02:43:12 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416507040 data=559201bc6461677372000002>, participation_id: <BSON::ObjectId:0x70194418024680 data=5591fdd96461677062000003>>=>
  #<FReCon::Match _id: 559201bc6461677372000002, created_at: 2015-06-30 02:41:00 UTC, updated_at: 2015-06-30 02:41:00 UTC, number: "qm1", blue_score: 0, red_score: 0, competition_id: <BSON::ObjectId:0x70194416506660 data=5591fd886461677062000001>>}

# {FReCon::Record => FReCon::Competition, FReCon::Record => FReCon::Competition, FReCon::Record => FReCon::Competition, FReCon::Record => FReCon::Competition}
=> {#<FReCon::Record _id: 559201ff6461677372000004, created_at: 2015-06-30 02:42:07 UTC, updated_at: 2015-06-30 02:42:07 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416592780 data=559201bc6461677372000002>, participation_id: <BSON::ObjectId:0x70194418028800 data=5591fdd96461677062000003>>=>
  #<FReCon::Competition _id: 5591fd886461677062000001, created_at: 2015-06-30 02:23:04 UTC, updated_at: 2015-06-30 02:23:04 UTC, location: "Bananavilll", name: "IRI">,
 #<FReCon::Record _id: 5592020d6461677372000005, created_at: 2015-06-30 02:42:21 UTC, updated_at: 2015-06-30 02:42:21 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416563660 data=559201c66461677372000003>, participation_id: <BSON::ObjectId:0x70194418026960 data=5591fdd96461677062000003>>=>
  #<FReCon::Competition _id: 5591fd886461677062000001, created_at: 2015-06-30 02:23:04 UTC, updated_at: 2015-06-30 02:23:04 UTC, location: "Bananavilll", name: "IRI">,
 #<FReCon::Record _id: 559202386461677372000006, created_at: 2015-06-30 02:43:04 UTC, updated_at: 2015-06-30 02:43:04 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416544720 data=559201c66461677372000003>, participation_id: <BSON::ObjectId:0x70194418025820 data=5591fdd96461677062000003>>=>
  #<FReCon::Competition _id: 5591fd886461677062000001, created_at: 2015-06-30 02:23:04 UTC, updated_at: 2015-06-30 02:23:04 UTC, location: "Bananavilll", name: "IRI">,
 #<FReCon::Record _id: 559202406461677372000007, created_at: 2015-06-30 02:43:12 UTC, updated_at: 2015-06-30 02:43:12 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416507040 data=559201bc6461677372000002>, participation_id: <BSON::ObjectId:0x70194418024680 data=5591fdd96461677062000003>>=>
  #<FReCon::Competition _id: 5591fd886461677062000001, created_at: 2015-06-30 02:23:04 UTC, updated_at: 2015-06-30 02:23:04 UTC, location: "Bananavilll", name: "IRI">}

# {FReCon::Record => String, FReCon::Record => String, FReCon::Record => String, FReCon::Record => String}
=> {#<FReCon::Record _id: 559201ff6461677372000004, created_at: 2015-06-30 02:42:07 UTC, updated_at: 2015-06-30 02:42:07 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416592780 data=559201bc6461677372000002>, participation_id: <BSON::ObjectId:0x70194418028800 data=5591fdd96461677062000003>>=>
  "IRI",
 #<FReCon::Record _id: 5592020d6461677372000005, created_at: 2015-06-30 02:42:21 UTC, updated_at: 2015-06-30 02:42:21 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416563660 data=559201c66461677372000003>, participation_id: <BSON::ObjectId:0x70194418026960 data=5591fdd96461677062000003>>=>
  "IRI",
 #<FReCon::Record _id: 559202386461677372000006, created_at: 2015-06-30 02:43:04 UTC, updated_at: 2015-06-30 02:43:04 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416544720 data=559201c66461677372000003>, participation_id: <BSON::ObjectId:0x70194418025820 data=5591fdd96461677062000003>>=>
  "IRI",
 #<FReCon::Record _id: 559202406461677372000007, created_at: 2015-06-30 02:43:12 UTC, updated_at: 2015-06-30 02:43:12 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416507040 data=559201bc6461677372000002>, participation_id: <BSON::ObjectId:0x70194418024680 data=5591fdd96461677062000003>>=>
  "IRI"}

# [FReCon::Record => boolean (true if element is selected, false if not), FReCon::Record => boolean (true if element is selected, false if not), FReCon::Record => boolean (true if element is selected, false if not), FReCon::Record => boolean (true if element is selected, false if not)]
=> {#<FReCon::Record _id: 559201ff6461677372000004, created_at: 2015-06-30 02:42:07 UTC, updated_at: 2015-06-30 02:42:07 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416592780 data=559201bc6461677372000002>, participation_id: <BSON::ObjectId:0x70194418028800 data=5591fdd96461677062000003>>=>
  true,
 #<FReCon::Record _id: 5592020d6461677372000005, created_at: 2015-06-30 02:42:21 UTC, updated_at: 2015-06-30 02:42:21 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416563660 data=559201c66461677372000003>, participation_id: <BSON::ObjectId:0x70194418026960 data=5591fdd96461677062000003>>=>
  true,
 #<FReCon::Record _id: 559202386461677372000006, created_at: 2015-06-30 02:43:04 UTC, updated_at: 2015-06-30 02:43:04 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416544720 data=559201c66461677372000003>, participation_id: <BSON::ObjectId:0x70194418025820 data=5591fdd96461677062000003>>=>
  true,
 #<FReCon::Record _id: 559202406461677372000007, created_at: 2015-06-30 02:43:12 UTC, updated_at: 2015-06-30 02:43:12 UTC, notes: nil, position: "r1", match_id: <BSON::ObjectId:0x70194416507040 data=559201bc6461677372000002>, participation_id: <BSON::ObjectId:0x70194418024680 data=5591fdd96461677062000003>>=>
  true}
rye commented 9 years ago

Looking at how I generated that code, a potential parsing algorithm could be this:

def self.parse_string(collection = Team.find_by(number: 2503).records, params = {"match+competition+name" => "IRI"})
    psv_query_string = params.keys.first
    # => "match+competition+name"

    final_check_value = params[psv_query_string]
    # => "IRI"

    field_query_elements = psv_query_string.split("+").map do |psv_element|
        psv_element.to_sym
    end
    # => [:match, :competition, :name]

    elements = {}
    # => {}

    # Pre-seed the `elements` hash.
    collection.each do |element|
        elements[element] = element
    end

    # Loop through each field query element (e.g. [:match, :competition, :name])
    field_query_elements.each do |field_query_element|
        # Loop through all of the key-value pairs in `elements`
        elements.each do |collection_element, value|
            # Replace the value with value's result after sending it `field_query_element`.
            elements[collection_element] = value.send(field_query_element)
        end
    end

    # `elements` is now a key-value mapping of the original record to the value
    # that will be compared, which, with the example, is a mapping of
    # FReCon::Records to Strings.
    #
    # Do the comparison between all of the values and the final check value, then
    # select only the true ones, and return the array of the keys in that hash.
    matching_elements = elements.each do |collection_element, value|
        elements[collection_element] = (value == final_check_value)
    end.select do |collection_element, value|
        value
    end.map do |collection_element, value|
        collection_element
    end

    matching_elements
end

Of course, I haven't actually checked this, but it seems like it should work. It also seems like this will minimize the number of queries that are necessary.

Comments?

rye commented 9 years ago

I've made some edits to it, now.

@Sammidysam, how does this sounds to you? The huge advantage of this that you can basically whittle down everything: match+competition+name runs #match on each Record, then #competition on each of those results, then #name on each of those results. It then checks each of the results against the desired result (value == "IRI"), and then returns only the correct ones.

These are big queries, and we'd probably not do them all to often, but I think it's a nice feature to have in the API.

Also, this could easily be made recursive. Here's an example for that:

def self.parse_string(collection = Team.find_by(number: 2503).records, params = {"match+competition+name" => "IRI"})
    elements = {}
    # => {}

    matching_elements = []

    params.each do |key, final_value|
        # Pre-seed the `elements` hash.
        collection.each do |element|
            elements[element] = element
        end

        attribute_query_elements = key.split("+").map do |query_element|
            query_element.to_sym
        end
        # => [:match, :competition, :name]

        # Loop through each attribute query element (e.g. [:match, :competition, :name])
        attribute_query_elements.each do |attribute_query_element|
            # Loop through all of the key-value pairs in `elements`
            elements.each do |collection_element, value|
                # Replace the value with value's result after sending it `attribute_query_element`.
                elements[collection_element] = value.send(attribute_query_element)
            end
        end

        # `elements` is now a key-value mapping of the original record to the value
        # that will be compared, which, with the example, is a mapping of
        # FReCon::Records to Strings.
        #
        # Do the comparison between all of the values and the final check value, then
        # select only the true ones, and return the array of the keys in that hash.
        param_matching_elements = elements.each do |collection_element, value|
            elements[collection_element] = (value == final_value)
        end.select do |collection_element, value|
            value
        end.map do |key, value|
            key
        end

        matching_elements.concat(param_matching_elements)
    end

    matching_elements.uniq
end
rye commented 9 years ago

The above code samples will need to be majorly refactored before incorporation, largely to include error checking and some huge performance improvements. (E.g. only check the results from the previous param's checking, don't check things over and over again and then #uniq them)

Sammidysam commented 9 years ago

Why was where not used? I guess I'm just confused by what the string truly does.

-Sam

Sent using CloudMagic [https://cloudmagic.com/k/d/mailapp?ct=pa&cv=6.3.45&pv=5.0.2]

On Wed, Jul 01, 2015 at 12:32 AM, Kristofer Rye < notifications@github.com [notifications@github.com] > wrote: The above code samples will need to be majorly refactored before incorporation, largely to include error checking and some huge performance improvements. (E.g. only check the results from the previous param's checking, don't check things over and over again and then #uniq them)

— Reply to this email directly or view it on GitHub [https://github.com/frc-frecon/frecon/pull/60#issuecomment-117432141] .[https://github.com/notifications/beacon/AC7z1us7QB_mG99EZAt-FpAQCGlUdkTrks5oY2TwgaJpZM4FOgLC.gif]

Sammidysam commented 9 years ago

Oh I didn't see your edits. That makes sense now and it does look helpful, but you should watch out for preventing n+1 queries in the model selection. I also am still confused why the where method was not used, but I couldn't read the code all that well on my phone.

-Sam

Sent using CloudMagic [https://cloudmagic.com/k/d/mailapp?ct=pa&cv=6.3.45&pv=5.0.2]

On Wed, Jul 01, 2015 at 7:58 AM, Sam Craig < sammidysam@gmail.com [sammidysam@gmail.com] > wrote: Why was where not used? I guess I'm just confused by what the string truly does.

-Sam

Sent using CloudMagic [https://cloudmagic.com/k/d/mailapp?ct=pa&cv=6.3.45&pv=5.0.2]

On Wed, Jul 01, 2015 at 12:32 AM, Kristofer Rye < notifications@github.com [notifications@github.com] > wrote: The above code samples will need to be majorly refactored before incorporation, largely to include error checking and some huge performance improvements. (E.g. only check the results from the previous param's checking, don't check things over and over again and then #uniq them)

— Reply to this email directly or view it on GitHub [https://github.com/frc-frecon/frecon/pull/60#issuecomment-117432141] .[https://github.com/notifications/beacon/AC7z1us7QB_mG99EZAt-FpAQCGlUdkTrks5oY2TwgaJpZM4FOgLC.gif]

rye commented 9 years ago

The difference here is that the API can select objects that match a given attribute or attribute chain, not just an attribute. I'm not aware of how that can be done with where.

On Wed, Jul 1, 2015, 07:01 Sam Craig notifications@github.com wrote:

Oh I didn't see your edits. That makes sense now and it does look helpful, but you should watch out for preventing n+1 queries in the model selection. I also am still confused why the where method was not used, but I couldn't read the code all that well on my phone.

-Sam

Sent using CloudMagic [ https://cloudmagic.com/k/d/mailapp?ct=pa&cv=6.3.45&pv=5.0.2]

On Wed, Jul 01, 2015 at 7:58 AM, Sam Craig < sammidysam@gmail.com [ sammidysam@gmail.com] > wrote: Why was where not used? I guess I'm just confused by what the string truly does.

-Sam

Sent using CloudMagic [ https://cloudmagic.com/k/d/mailapp?ct=pa&cv=6.3.45&pv=5.0.2]

On Wed, Jul 01, 2015 at 12:32 AM, Kristofer Rye < notifications@github.com [notifications@github.com] > wrote: The above code samples will need to be majorly refactored before incorporation, largely to include error checking and some huge performance improvements. (E.g. only check the results from the previous param's checking, don't check things over and over again and then #uniq them)

— Reply to this email directly or view it on GitHub [https://github.com/frc-frecon/frecon/pull/60#issuecomment-117432141] .[ https://github.com/notifications/beacon/AC7z1us7QB_mG99EZAt-FpAQCGlUdkTrks5oY2TwgaJpZM4FOgLC.gif]

— Reply to this email directly or view it on GitHub https://github.com/frc-frecon/frecon/pull/60#issuecomment-117633913.

Sammidysam commented 9 years ago

Just the main problem is that if you have 5 records, I feel like this method will require at least 11 queries (1 for the 5 records, 5 for each match, and 5 for each competition). I may be wrong, but I feel like it does not really narrow down the match and competition queries to one each. This is important for here, where we want speed.

rye commented 9 years ago

Looking at my log files, it looks like you're somewhat right about the higher number of queries.

I still don't just want to pass the params hash to where, because that won't let us test nested attributes, which is a big help from these sample things. These code samples might not necessarily work the best, but having the ability to filter by not only a single attribute but a chain of attributes.