6
\$\begingroup\$

This is my first Rails project. I have read about concerns which are a great tool, but I am confused about how I should redesign my models.

Some models have similar fields, but the validations are not the same everywhere. For example: 3 models have a phone field, but in one model the phone field is not required. 3 models have a name field, but the Partner model doesn't. I am not sure about how I should organize my code.

Models

Event

class Event < ActiveRecord::Base
    include NameFormField, PhoneFormField, LocaleFormField
    include SetUserAssociation, SetCountyAssociation

    belongs_to  :school

    validates   :phone,         presence: true
    validates   :description,   presence: true    
    validates   :event_start,   presence: true
    validates   :event_end,     presence: true
end

Partner

class Partner < ActiveRecord::Base
    include PhoneFormField
    include SetUserAssociation, SetCountyAssociation

    belongs_to :partner_type, class_name: 'Common::PartnerType'
    belongs_to :dance_level,  class_name: 'Common::DanceLevel'
    belongs_to :dance_style,  class_name: 'Common::DanceStyle'
    belongs_to :dance_goal,   class_name: 'Common::DanceGoal'

    validates :email,       presence: true
    validates :city,        presence: true
    validates :dance_style, presence: true
    validates :dance_level, presence: true
    validates :dance_goal,  presence: true
    validates :age,         numericality: { less_than_or_equal_to: 120,  only_integer: true }
    validates :height,      numericality: { less_than_or_equal_to: 300,  only_integer: true }, allow_blank: true
    validates :weight,      numericality: { less_than_or_equal_to: 100,  only_integer: true }, allow_blank: true
    validates :club,        length: { maximum: 100, too_long: "%{count} characters are allowed." }, allow_blank: true
    validates :weekly,      numericality: { less_than_or_equal_to: 7,  greater_than_or_equal_to: 1, only_integer: true }, allow_blank: true
    validates :occasionally,numericality: { less_than_or_equal_to: 5,  greater_than_or_equal_to: 1, only_integer: true }, allow_blank: true
    validates :years,       numericality: { less_than_or_equal_to: 50, greater_than_or_equal_to: 0, only_integer: true }, allow_blank: true
end

School

class School < ActiveRecord::Base
    include NameFormField, PhoneFormField, LocaleFormField
    include SetUserAssociation, SetCountyAssociation

    has_many :schools_dance_styles, class_name: 'Common::SchoolsDanceStyle'
    has_many :dance_styles, dependent: :destroy, through: :schools_dance_styles, class_name: 'Common::DanceStyle'

    validates :phone,       presence: true
    validates :dance_styles,presence: true
    validates :description, presence: true
end

School Review:

class SchoolReview < ActiveRecord::Base
    include NameFormField, SetUserAssociation

    belongs_to :school

    validates :description, presence: true 
    validates :rate, presence: true, numericality: { less_than_or_equal_to: 5,  greater_than_or_equal_to: 1, only_integer: true }
end

Concerns:

module NameFormField
  extend ActiveSupport::Concern

  included do
    validates :name, presence: true
  end
end

module PhoneFormField
  extend ActiveSupport::Concern

  included do
    validates :phone, length: { maximum: 20, too_long: "%{count} characters are allowed"}  
  end

  def phone=(value)
       super(value.blank? ? nil : value.gsub(/\D/, '')) 
    end
end

module LocaleFormField
  extend ActiveSupport::Concern

  included do
    validates :city,    presence: true
    validates :address, presence: true
  end
end

module SetCountyAssociation
  extend ActiveSupport::Concern

  included do
    belongs_to :county, class_name: 'Common::County'

    validates  :county, presence: true
  end
end

module SetUserAssociation
  extend ActiveSupport::Concern

  included do
    belongs_to :user

    validates  :user, presence: true
  end
end

module SetSchoolAssociation
  extend ActiveSupport::Concern

  included do
    belongs_to :school

    validates  :school, presence: true
  end
end

Can you give me advice about how to organize my code? It seems all models can inherit from a "UserBased" model, because all models need a belongs_to relationship to the User model.

Here an "UML" diagram. The red means actual field is required! (For example in the Partner the phone and description are not required!) enter image description here

\$\endgroup\$
  • \$\begingroup\$ Could you add a high-level overview of your models? I.e. what belongs to what. Seems to me that you have quite a few models, and making heads and tails of it from code alone isn't super easy without some basic introduction \$\endgroup\$ – Flambino Nov 15 '15 at 22:25
  • \$\begingroup\$ Okay Flambino, just gives me 12 hours :) \$\endgroup\$ – Bóta László Nov 15 '15 at 23:10
  • \$\begingroup\$ No rush :) I just think your question deserves an answer/review, but with a big project it's a little daunting for reviewers to just jump in, so the easier it is to get going, the better \$\endgroup\$ – Flambino Nov 15 '15 at 23:14
  • \$\begingroup\$ I've done an "UML" diagram, so you can see what belongs to what. (And the required fields too, those are highlighted with red color.) \$\endgroup\$ – Bóta László Nov 16 '15 at 0:28
2
\$\begingroup\$

To be honest, simple validations like this are not good candidates for concerns because of the additional level of indirection introduced by the concerns themselves. Go for clarity in this case rather than cleverness. Concerns are "clever" in most cases, meaning they are more than what you really need. For your use case, I think concerns are like hitting a thumbtack with a sledge hammer.

\$\endgroup\$
  • \$\begingroup\$ Honestly, I agree with you. I started to refactor this part and many concerns were dropped out. \$\endgroup\$ – Bóta László Nov 18 '15 at 17:16
  • \$\begingroup\$ @BótaLászló if you like this answer, perhaps you can give it a vote ;-) \$\endgroup\$ – Stop ongoing harm to Monica Dec 23 '16 at 14:20

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.