3
\$\begingroup\$

This is a class from my rails application. It is used as non-database model in my RoR application and uses syntax similar to AR design pattern. I wonder is there a way to make this look more like Ruby code?

class Role

  def self.all
    role_list = Array.new

    r1 = Role.new
    r1.id = 1
    r1.symbol = :owner
    r1.name = "Owner"
    r1.has_all_permissions = true
    role_list << r1

    r2 = Role.new
    r2.id = 2
    r2.symbol = :admin
    r2.name = "Administrator"
    r2.has_all_permissions = false
    role_list << r2

    r3 = Role.new
    r3.id = 3
    r3.symbol = :gamemaster
    r3.name = "Gamemaster"
    r3.has_all_permissions = false
    role_list << r3

    role_list
  end

  def self.find(id)
    roles = Role.all

    roles.each do |role|
      if role.id == id
        return role
      end
    end

    nil
  end

  def self.find_by_symbol(symbol)
    roles = Role.all

    roles.each do |role|
      if role.symbol == symbol
        return role
      end
    end

    nil
  end

  def assign_permission(symbol)
    permission = Permission.find_by_symbol(symbol)

    if permission == nil
      return nil
    end

    permission_roles = PermissionRole.where(:permission_id => permission.id, :role_id => self.id)

    if permission_roles.size == 0
      permission_role = PermissionRole.new
      permission_role.permission_id = permission.id
      permission_role.role_id = self.id
      return permission_role.save
    end

    true
  end

  def has_permission?(symbol)
    permission = Permission.find_by_symbol(symbol)

    if permission == nil
      raise StandardError, "Permission does not exist"
    end

    permission_roles = PermissionRole.where(:permission_id => permission.id, :role_id => self.id)

    if permission_roles.size > 0
      return true
    else
      return false
    end
  end

  attr_accessor :id
  attr_accessor :symbol
  attr_accessor :name
  attr_accessor :has_all_permissions

end
\$\endgroup\$
3
\$\begingroup\$

For starters:

def initialize(attributes = {})
  attributes.each do |attr, value|
    self.send "#{attr}=", value
  end
end

Would allow you to turn:

r1 = Role.new
r1.id = 1
r1.symbol = :owner
r1.name = "Owner"
r1.has_all_permissions = true
role_list << r1

into:

role_list << Role.new {:id => 1,
                       :symbol => :owner,
                       :name => "Owner",
                       :has_all_permissions => true}

which is much more ActiveRecord-like.


Storing the roles_list in a global variable might be acceptable for your application and would also allow you to have:

def self.create(attributes = {})
  $roles_list << Role.new(attributes)
end

and:

def self.all
  return $roles_list if $roles_list

  # rest of code to init $roles_list with default roles
end

for further ActriveRecord-ness


Then:

def self.find(id)
  roles = Role.all

  roles.each do |role|
    if role.id == id
      return role
    end
  end

  nil
end

could be re-written like so:

def self.find(id)
  Role.all.find{|r| r.id == id}
end

That is because Array has its own #find method. The same refactoring could be applied to self.find_by_symbol.


As a final note, I would move the attr_accessor lines to the top.

Hope I helped :-)

\$\endgroup\$
  • \$\begingroup\$ You could also include ActiveModel in this to give it a nice AR-style interface even without a DB connection. \$\endgroup\$ – Marnen Laibow-Koser Oct 21 '11 at 21:31

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.