3
\$\begingroup\$

I'm pretty new to web development and I don't know if my solution is OK according to the best practices, considering performance and clean code.

I have a Merchant model that belongs to a Price model.

Price table:

create_table "prices", force: :cascade do |t|
  t.string   "price_range"
  t.integer  "percentile"
  t.datetime "created_at",  null: false
  t.datetime "updated_at",  null: false
end

The current values inside the table:

**price_range**    **percentile **
Up to $20.00            20
$20.01 to $40.00        40
$40.01 to $60.00        60
$60.01 to $80.00        80
More than $80.00       100

To show this at my view I'm using:

<div class="price-ratings">
  <div class="empty-prices"></div>
  <div class="full-prices" style= width:"<%= "#{@merchant.price.percentile}%" %></div>
</div>

According to the width value the dollar signals will be colored (the CSS part of code is in the jsFiddle link).

jsFiddle

I'm not sure about that approach. Can anyone recommend a finer solution to achieve the same or is that OK?

\$\endgroup\$
1
\$\begingroup\$

Using a string column is not a very ideal solution. How would you for example query for rows where the price range is within or below 70.00$?

What if you want to do calculations in the database based on the values?

Instead you should use two columns:

create_table "prices", force: :cascade do |t|
  t.decimal   "price_min"
  t.decimal   "price_max"
  t.integer  "percentile"
  t.datetime "created_at",  null: false
  t.datetime "updated_at",  null: false
end

Note that when dealing with money you should use an integer in the lowest unit of currency (cents) or a decimal. See: why not use Double or Float to represent currency?

You can even write a methods in your model to get/set price_range with a range:

class Price
  def price_range
    (price_min..price_max)
  end 

  def price_range=(range)
    self[:price_min], self[:price_max] = range.begin, range.end
  end
end

If you are using Postgres you can use the native range database type.

create_table :events do |t|
  t.numrange :price_range
  t.integer  "percentile"
  t.datetime "created_at",  null: false
  t.datetime "updated_at",  null: false
end

You can write a helper method to format it nicely

module PriceHelper
  def price_range_human( range )
    "$%s to $%s" % [range.begin, range.end].map(:number_to_currency)
  end
end

See ActionView::Helpers::NumberHelper

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.