HiveBrain v1.2.0
Get Started
← Back to all entries
principlejavascriptrailsMinor

Price rating approach

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
priceratingapproach

Problem

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:







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?

Solution

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

Code Snippets

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
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
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
module PriceHelper
  def price_range_human( range )
    "$%s to $%s" % [range.begin, range.end].map(:number_to_currency)
  end
end

Context

StackExchange Code Review Q#143933, answer score: 2

Revisions (0)

No revisions yet.