-1

I have two models:

class Order < ActiveRecord::Base
  belongs_to :user
  has_one :order_type
end

class OrderType < ActiveRecord::Base
  belongs_to :order
end

my schema.rb:

create_table "order_types", force: :cascade do |t|
  t.string   "ort_name"
  t.datetime "created_at", null: false
  t.datetime "updated_at", null: false
end

create_table "orders", force: :cascade do |t|
  t.string   "ord_name"
  t.date     "ord_due_date"
  t.integer  "user_id"
  t.integer  "ordertype_id"
  t.datetime "created_at",   null: false
  t.datetime "updated_at",   null: false
end

add_index "orders", ["ordertype_id"], name: "index_orders_on_ordertype_id"
add_index "orders", ["user_id"], name: "index_orders_on_user_id"

There is only one-direction association between them. The Order model has a column "ordertype_id" that links to the appropriate order_type.

My question is, what is the best practice to access the ort_name value for each @order in a view.

Currently, I am using:

<p> 
  <strong>Ord type:</strong>
  <% OrderType.where(id:  @order.ordertype_id).each do |t| %>
  <%= t.ort_name %>
  <% end %>
</p>

This solution results in many code repetitions. How I should change that? Can somebody advise, as I am not so experienced yet?

I tried this code, but it did not work:

@orders.order_type
davegson
  • 8,205
  • 4
  • 51
  • 71
Michal
  • 139
  • 3
  • 14
  • what is the difference between `ord_name` and `ort_name`? It seems very bad practice to create a model for a single attribute... – davegson Sep 07 '15 at 13:41
  • Imagine such scenario: you have only 5 types of orders that can be picked up by a user while creating an order. But in the future you want to have an option to extend that list (for new orders). In my opinion it is correct to have a separate table that keeps available values of order types. – Michal Sep 07 '15 at 13:51

2 Answers2

1

There are many problems which you should address. It's ok to be a beginner, just take yourself time to learn and improve.

Schema

First off, your schema is set up badly. If you want to limit the order type to certain values, you should do this with a validation.

class Order
  TYPES = %w[foo bar three four five]

  validates :order_type, inclusion: { in: TYPES }
end

This way, you can easily add values in the future, and remove the complexity of adding a new model and its relations.

Column Names

Secondly, you should revise your column names. ord_name and ord_due_date is bad, it leads to ugly calls like order.ord_name. You should drop the prefix ord, it's superfluous.

Both steps would lead to this schema.rb

create_table "orders", force: :cascade do |t|
  t.string   "name"
  t.date     "due_date"
  t.integer  "user_id"
  t.string   "order_type"
  t.datetime "created_at",   null: false
  t.datetime "updated_at",   null: false
end

Logic placement

My final advice is to never call queries from your view. Logic should always be in the controller / model & passed to the view via instance variables.

This is a big no no in rails:

<% OrderType.where(id:  @order.ordertype_id).each do |t| %>
  ...
<% end %>

In the end, accessing the type is simply accomplished with:

@order.order_type
davegson
  • 8,205
  • 4
  • 51
  • 71
  • @Michal, I'd ask you to accept my answer, since you are [obviously using my solution](http://stackoverflow.com/questions/32445865/rails-collection-select-populate-with-the-values-listed-in-a-model). – davegson Sep 08 '15 at 09:09
-1

Update your Order model to this:

class Order < ActiveRecord::Base
  belongs_to :user
  has_one :order_type, foreign_key: 'ordertype_id`
end

then order_type should be easily accessible:

@order.order_type.ort_name
dimakura
  • 7,575
  • 17
  • 36