launchscout / nku

NKU Class Spring
5 stars 14 forks source link

Question about STIs or polymorphic associations #89

Closed staffordw1 closed 10 years ago

staffordw1 commented 10 years ago

I have two models so far but I plan on adding more. My first model is a Medium model (apparently medium is the singular of media) and this has fields such as year, title, image_url and I also added type because I read about STIs and they said that a type field is automatically updated by ruby to associate another model with this one. My second model is a Game model.

So what I want to do is use the Medium model as a sort of abstract model. I want to have the models: games, books, tv and movies each with their own fields but I want them to also inherit the fields from the Medium model. Since my app idea counts on being able to query the single Media table for similar attributes it's kind of necessary for this relationship to be in place.

What I tried to do was implement an STI:

#medium.rb
class Medium < ActiveRecord::Base
end
#game.rb
class Game < Medium
end

And I figured that was all that was needed to allow the association. I added an index page with a link to new_game_path.

My form breaks though; it allows me to create a game but only with the fields from the Medium model. I can't use any of the fields from the Game model. Does anyone know how I can fix this?

jaimerump commented 10 years ago

Have you tried creating a new game from the console instead? If it fails in the console as well, that means that the problem is in the model or database. If it works, then the problem is probably specifically with the form.

staffordw1 commented 10 years ago

Yep, I'm allowed to create a new game from the console but with the same problem as before as creating a game as a Medium with only the Medium attributes. Here is the form I'm using:

<% flash.each do |name, msg| -%>
      <%= content_tag :div, msg, class: name %>
<% end -%>
<h1>Create New Game</h1>
<%= form_for @game, url: media_path do |f| %>
  <% if @game.errors.any? %>
      <h2>Form is invalid</h2>
      <ul>
        <% for message in @game.errors.full_messages %>
          <li><%= message %></li>
        <% end %>
      </ul>
  <% end %>
  <p>
    <%= f.label :title %><br>
    <%= f.text_field :title %>
  </p>

  <p>
    <%= f.label :year %><br>
    <%= f.text_field :year %>
  </p>

  <p>
    <%= f.label :series %><br>
    <%= f.text_field :series %>
  </p>

  <p>
    <%= f.label :image_url %><br>
    <%= f.text_field :image_url %>
  </p>
  <p>
    <%= f.label :publisher %><br>
    <%= f.text_field :publisher %>
  </p>

  <p>
    <%= f.label :developer %><br>
    <%= f.text_field :developer %>
  </p>

  <p>
    <%= f.label :genre %><br>
    <%= f.text_field :genre %>
  </p>

  <p>
    <%= f.label :platform %><br>
    <%= f.text_field :platform %>
  </p>
  <p>
    <%= f.submit %>
  </p>
<% end %>
<%= link_to 'Back', media_path %>

It gives me an undefined method error for :publisher. Every attribute above it is in the Medium table whereas publisher and the attributes that follow are in the Game table.

jaimerump commented 10 years ago

If you're getting the same error from the console, then the form isn't the issue, there's something messed up either in the relation between the two models, or the relation between them and the database.

I notice that in the latest version of your repo on github, you have separate tables for games and media. I would guess that that's the issue, since STI stands for Single Table Inheritance.

staffordw1 commented 10 years ago

Hmm, so you're saying I can combine the two tables but somehow keep the attributes I need only for game separate from the rest of the attributes in the Medium table? Or does STI require that I only have attributes in my Medium table? If that's the case, should I be using a different sort of inheritance?

jaimerump commented 10 years ago

I'm not familiar with the STI model, but everything I'm reading suggests that you combine all of the attributes of all of the child classes into one table, throw in a type flag so that Rails knows which model to use for each row, and then only populate the relevant columns for each row.

That works pretty well when you have a very small number of classes, and they share all but a few columns. I'm not sure if it's appropriate in your case or not, since you have a 4 classes, and they have very few attributes in common. @mitchlloyd or @rockwood would be able to give a better answer there.

jaimerump commented 10 years ago

If you haven't already seen it, this is the Rails documentation for STI http://api.rubyonrails.org/classes/ActiveRecord/Base.html#label-Single+table+inheritance

Your case might be closer to polymorphic association which is pretty similar to implementing an interface. http://guides.rubyonrails.org/association_basics.html#polymorphic-associations

staffordw1 commented 10 years ago

Alright, that's something to look into, thanks! Somehow none of the tutorials I read about mentioned anything like that, or I just didn't notice it.

I was planning on adding more attributes later once I had a better idea of how everything would all fit together.

Yeah I was looking at polymorphic associations, from the examples I've seen, STI looks more like what I'm trying to do and it seemed simple enough so I went for that. Polymorphic associations look a bit more complicated to implement, and it looks like that would create 4 separate tables when what I want in the long run is one overarching table to query so that I can find similarities between all 4 things... anyways I'll close the issue for now and see if I can work something out, thanks again!

mitchlloyd commented 10 years ago

I don't have time for an in-depth response right now but I just want to say that STI should almost never be used.

On Friday, April 4, 2014, staffordw1 notifications@github.com wrote:

Closed #89 https://github.com/gaslight/nku/issues/89.

Reply to this email directly or view it on GitHubhttps://github.com/gaslight/nku/issues/89 .

staffordw1 commented 10 years ago

OK , I'll look into other options

mitchlloyd commented 10 years ago

As a follow up: I've never seen STI implemented in a way that hasn't been regretted. One core problem is that inheritance in general can be a trap, especially given the way it is conventionally taught and discussed: (see http://lists.canonical.org/pipermail/kragen-tol/2011-August/000937.html). A good rule of thumb for inheritance is that every subclass should be able to stand in for another (Liskov Substitution Principle). So unless a game has all of the same attribute as a book, I wouldn't use inheritance.

Composition is always an alternative to inheritance and usually produces better results. I would suggest letting go of concrete ideas like "a game is a medium". From what I've heard so far here are some better options than STI: