Topic: this method makes me sad

Hello,
I have a project class and a project_user class to allow a list of users when project is private.
If i write the allowed_users method like this:
(i don't know if this is a good way to do this by the way)

def allowed_user?(user)
  user.admin || user == self.created_by || user == self.project_users.find(:first, :conditions => "user_id = '#{user.id}'").user
end

But it returns nil instead of false if the user does not match the conditions.

So i have to write it this way:

def allowed_user?(user)
  if user.admin || user == self.created_by
    true
  elsif self.project_users.find(:first, :conditions => "user_id = '#{user.id}'")
    if  user == self.project_users.find(:first, :conditions => "user_id = '#{user.id}'").user
      return true
    end
  else
    false
  end
end

But this is too long and dirty and even a noob like myself  knows this is not the way smile
How can i do this shorter?

Re: this method makes me sad

What's wrong with nil?

You could do

def allowed_user?(user)
  return true if user.admin || user == self.created_by || user == self.project_users.find(:first, :conditions => "user_id = '#{user.id}'").user
  false
end

Re: this method makes me sad

when i test it like this

project = create(:private => true, :created_by => @user)
project.project_users.create(:user => @user2)
assert_equal(project.allowed_user?(@user3), false)

it throws this. for that nil value.

NoMethodError : You have nil object when you didn't expect it!

thank you for answer but it still gives the same error

Last edited by imit (2009-07-08 13:24:26)

Re: this method makes me sad

assert !project.allowed_user?(@user3)

and nil method error probably comes from this:
|| user == self.project_users.find(:first, :conditions => "user_id = '#{user.id}'").user

Just make:

user.admin || user == self.created_by || self.project_users.find(:first, :conditions => "user_id = '#{user.id}'")

Last edited by innu (2009-07-08 13:42:03)

Re: this method makes me sad

i narrowed down to this

def allowed_user?(user)
  pu ||= self.project_users.find(:first, :conditions => "user_id = '#{user.id}'")
  user.admin || user == self.created_by || user == pu.user
end

but it stills throws errors whining about the nil in tests or in console if the find methods returns nil

changing the test doesn't work for me. i need to return false if that "project_users.find" method returns nil.
how can i convert nil to a false?

Re: this method makes me sad

why are you checking user == pu.user. You search this user already with user.id so you don't need to double check it. If you really want to double check it then just do:
|| user == pu && pu.user

if pu is nil, then you are calling nil.user and you'll get nil error.

Just make your method like this:

def allowed_user?(user)
  self.project_users.find(:first, :conditions => "user_id = '#{user.id}'") ||
  user.admin ||
  user == self.created_by ||
end

In test you can do assert !project.allowed_user?(@user3)
In other places: if project.allowed_user(@user)

You don't need to convert nil to false for sure.
If you want to know how to convert it then:
!!nil #=> false
!!true #=> true

Last edited by innu (2009-07-08 17:48:46)

Re: this method makes me sad

innu wrote:

why are you checking user == pu.user. You search this user already with user.id so you don't need to double check it. If you really want to double check it then just do:
|| user == pu && pu.user

if pu is nil, then you are calling nil.user and you'll get nil error.

Just make your method like this:

def allowed_user?(user)
  self.project_users.find(:first, :conditions => "user_id = '#{user.id}'") ||
  user.admin ||
  user == self.created_by ||
end

In test you can do assert !project.allowed_user?(@user3)
In other places: if project.allowed_user(@user)

You don't need to convert nil to false for sure.
If you want to know how to convert it then:
!!nil #=> false
!!true #=> true

Those sql conditions make me sad. Don't interpolate string in your conditions.

find(:first, :conditions => ["user_id = ?", user.id]

is how you should do your conditions.

Re: this method makes me sad

innu wrote:

why are you checking user == pu.user. You search this user already with user.id so you don't need to double check it. If you really want to double check it then just do:
|| user == pu && pu.user

if pu is nil, then you are calling nil.user and you'll get nil error.

Just make your method like this:

def allowed_user?(user)
  self.project_users.find(:first, :conditions => "user_id = '#{user.id}'") ||
  user.admin ||
  user == self.created_by ||
end

In test you can do assert !project.allowed_user?(@user3)
In other places: if project.allowed_user(@user)

You don't need to convert nil to false for sure.
If you want to know how to convert it then:
!!nil #=> false
!!true #=> true

ah, ok i get it know. i learned lots. thanks smile

Re: this method makes me sad

cherring wrote:
innu wrote:

why are you checking user == pu.user. You search this user already with user.id so you don't need to double check it. If you really want to double check it then just do:
|| user == pu && pu.user

if pu is nil, then you are calling nil.user and you'll get nil error.

Just make your method like this:

def allowed_user?(user)
  self.project_users.find(:first, :conditions => "user_id = '#{user.id}'") ||
  user.admin ||
  user == self.created_by ||
end

In test you can do assert !project.allowed_user?(@user3)
In other places: if project.allowed_user(@user)

You don't need to convert nil to false for sure.
If you want to know how to convert it then:
!!nil #=> false
!!true #=> true

Those sql conditions make me sad. Don't interpolate string in your conditions.

find(:first, :conditions => ["user_id = ?", user.id]

is how you should do your conditions.

Re: this method makes me sad

Sorry that I'm a little late to the game, but this should work and provide the "false" you are looking for:

def allowed_user?(user)
  user.admin || user == self.created_by || self.project_users.include?(user)
end

The "include?" method on array objects checks to see if any of the elements of the array equals the passed in parameter. In this case, you are asking if your user is among the project_users of the project. It returns a boolean true or false.