Search code examples
ruby-on-railsactiverecordassociations

Is it generally bad practice to have 2 different associations between the same 2 objects?


I realize this is somewhat opinionated, so specifically asking whether or not this is generally a good idea per code convention.

Example scenario

Modeling the relationship between Books & Clients in a library, where let's assume that per library rules, a client can only ever checkout one book at a time.

What's making this complex for me is the fact that a client checks a book out only for a given period of time and then returns it. So it's almost like, you don't want that relationship to continue forever... at some point, a client is relieved of the responsibilities of that book (e.g., if a book got damaged 2 years after the client had returned it, you don't want to call up a past client asking for a replacement).

Yet although it's temporal, you still want a record of the relationship, so that you can track the history of clients who have checked out a book (because down the line, maybe you do want to send a survey to past clients asking for reviews).

In other words, it seems to me like there's 2 associations... one that's temporary, and one that's more lasting.

While I can technically write the code to make this work, it seems like terrible practice (like... intuitively I feel like the answer to this question is yes... that's not good practice). But I'd like to confirm, and see if so, what are other solutions to this issue (has_many :through maybe with the through table having some kind of... current_rental attribute?)

What 2 associations would look like

Association #1, the temporary association, you want to know which client a book is checked out to

Client has_one :book, foreign_key: "current_id"
Book belongs_to :current_client, class_name: "Client", foreign_key: "current_id"

This relationship is very useful, if client calls and says they forgot which book they had to return from their personal book rack, you can call @client.book.title.

Association #2, the longer term association, you want to know the history of a book (assume further that history of a client doesn't matter, if it did this would more clearly be a has_many :through or HABTM relationship)

Client belongs_to :checkout_book, class_name: "Book", foreign_key: "book_id"
Book has_many :clients, foreign_key: "book_id"

This relationship allows you to look at the history of a book, how many clients it's been checked out to @book.clients.size, or you can email them for a survey @book.clients.map(&:email).

This separation then removes the confusion where, say a book needs to be replaced, calling @book.clients.last may retrieve the incorrect client since reservations are made in advance and therefore checkouts don't necessarily happen in order. However, you can @book.current_client to get the right person to follow up with.


Solution

  • One reason you would would want to store an extra association is to optimize read queries if you have a joined table with a huge amount of data.

    Lets say that our hypothetical library is insanely popular and the are many thousands of loans per book. And we have it setup like so:

    class Client
      has_many :loans
      has_many :books, through: :loans
    end
    
    class Book
      has_many :loans
      has_many :clients, through: :loans
    end
    
    class Loan
      belongs_to :book
      belongs_to :client
    end
    
    class LoansController ApplicationController
      def create
        @book = Books.find(id: params[:book_id])
        @loan = @book.loans.new(client: current_client)
        if @loan.save
          redirect_to @loan
        else
          render :new           
        end
      end
    end
    

    We get the requirement to list the 100 most popular books and the client that is currently in possession of the book:

    class BooksController < ApplicationController
      def by_popularity
         @book = Book.order(popularity: :desc).limit(100)
      end
    end 
    
    <% @books.each do |book| %>
      <tr>
        <td><%= link_to book.title, book %></td>
        <td><%= link_to book.clients.last.name, book.clients.last %></td> 
      <tr>
    <% end %> 
    

    Since this causes a N+1 Query we get 101 database queries. Not good. So we throw includes on it to solve it.

    @book = Book.includes(:clients)
                .order(popularity: :desc)
                .limit(100)
    

    All good, right? Nope. For each of the records we are loading from books this will load and instantiate all the joined records from loans and clients. Since that's thousands and thousands the server grinds to a screeching halt as it runs out of memory.

    Although there are various tricks like sub-queries to limit the output the simplest and by far most performant solution is:

    class Book
      has_many :loans, after_add: :set_current_client
      has_many :clients, through: :loans
      belongs_to :current_client, class_name: 'Client'
    
      def set_current_client(client)
        update_column(current_client_id: current_client)
      end
    end
    
    class BooksController < ApplicationController
      def by_popularity
         @book = Book.order(popularity: :desc)
                     .eager_load(:current_client)
                     .limit(100)
      end
    end 
    
    <% @books.each do |book| %>
      <tr>
        <td><%= link_to book.title, book %></td>
        <td><%= link_to book.current_client.name, book.current_client %></td> 
      <tr>
    <% end %> 
    

    TLDR;

    Is it a bad practice to have multiple associations between the two same objects? No not necessarily. It is a legitimate solution to quite a few problems but does come with some cons of its own.