Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

undefined method `+' for nil:NilClass #278

Closed
wvteijlingen opened this issue May 31, 2017 · 5 comments
Closed

undefined method `+' for nil:NilClass #278

wvteijlingen opened this issue May 31, 2017 · 5 comments

Comments

@wvteijlingen
Copy link

I'm getting a NoMethodError (undefined method +' for nil:NilClass)in acts_as_list (0.9.5) lib/acts_as_list/active_record/acts/list.rb:400:in update_positions' when updating positions.

I first thought it had to do with null values in the position column, but this also occurs when values actually exist.

@brendon
Copy link
Owner

brendon commented May 31, 2017

Hi @wvteijlingen, I'm afraid we'll need a bit more information than that. Can you show us your class setup?

This is the failing line:

old_position = position_before_save || bottom_position_in_list + 1

If you can do some logging (download the gem locally and include it in your Gemfile manually) to see which of the two methods above is returning nil that would he helpful. Also what methods you execute to generate the error.

@wvteijlingen
Copy link
Author

wvteijlingen commented Jun 2, 2017

Sorry for the lack of information, I didn't have much time. I've looked into it now and found the problem. Turns out someone added a attr_accessor :position to our model. This caused the item.position call in the bottom_position_in_list method to always return nil. It's fixed in our codebase now.

However, maybe for the future it would be nice to catch this edge case in acts_as_list. Perhaps some logic like:

def bottom_position_in_list(except = nil)
  default_value = acts_as_list_top - 1
  item = bottom_item(except)
  if item
    item.send(position_column) || default_value
  else
    default_value
  end
end

Basically make sure that bottom_position_in_list always returns an integer, and never nil.

@brendon
Copy link
Owner

brendon commented Jun 2, 2017

Thanks @wvteijlingen. I don't think it's reasonable to guard against everything like that :) Sometimes mistakes like that one just slip in. Luckily, they're not without major errors most of the time which has led you to find a solution :) This issue will also help someone else in a similar position in the future :)

Thanks for following this up and keeping us updated :)

@brendon brendon closed this as completed Jun 2, 2017
@wvteijlingen
Copy link
Author

wvteijlingen commented Jun 2, 2017

No problem, I can understand.

I just think it would be good practice to make sure that a method will never return nil if it should return an integer. And if it can return nil for some reason, to have a check for that in the caller.

@brendon
Copy link
Owner

brendon commented Jun 5, 2017

I get what you're saying. In this case, barring something hacky like att_accessor overwriting a method, we can safely expect position_column to always return an integer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants