Lambda should change boolean values #16

Closed
fillman opened this Issue Feb 1, 2012 · 3 comments

Comments

Projects
None yet
2 participants
@fillman

fillman commented Feb 1, 2012

It seems like this wont work:
(debugging shows that lambda { simply_another_user.send :active } calls 2 times in @actual_before and @actual_after but returns true anyway even after calling a block). Any ideas?

if "should toggle a user" do
    simply_another_user = Factory(:user)
    simply_another_user.active.should be_true

    # Toggle from true to false
    debugger
    lambda {
      post :toggle, :id => simply_another_user
    }.should change(simply_another_user, :active)
end
@dchelimsky

This comment has been minimized.

Show comment Hide comment
@dchelimsky

dchelimsky Feb 1, 2012

Owner

The post affects the underlying data, but not the simply_another_user object. You could do this instead:

lambda {
  post :toggle, :id => simply_another_user
}.should change { simply_another_user.reload.active }

I'd be more inclined to do something like this:

user = Factory(:user, :active => true)
post :toggle, :id => user
User.find(user.id).active.should be_false
Owner

dchelimsky commented Feb 1, 2012

The post affects the underlying data, but not the simply_another_user object. You could do this instead:

lambda {
  post :toggle, :id => simply_another_user
}.should change { simply_another_user.reload.active }

I'd be more inclined to do something like this:

user = Factory(:user, :active => true)
post :toggle, :id => user
User.find(user.id).active.should be_false

@dchelimsky dchelimsky closed this Feb 1, 2012

@fillman

This comment has been minimized.

Show comment Hide comment
@fillman

fillman Feb 1, 2012

Thanks David, but why did you like the second variant? The first is much "readable" I think.

As I mentioned I came up with this:

lambda {
      post :toggle, :id => simply_another_user
}.should change { simply_another_user.reload.active }.from(true).to(false)

As it reads better. Thanks again David. Appreciate your work and time.

fillman commented Feb 1, 2012

Thanks David, but why did you like the second variant? The first is much "readable" I think.

As I mentioned I came up with this:

lambda {
      post :toggle, :id => simply_another_user
}.should change { simply_another_user.reload.active }.from(true).to(false)

As it reads better. Thanks again David. Appreciate your work and time.

@dchelimsky

This comment has been minimized.

Show comment Hide comment
@dchelimsky

dchelimsky Feb 1, 2012

Owner

"readability" is subjective, and you're not comparing the entire example. Compare these:

user = Factory(:user)
user.active.should be_true
lambda {
  post :toggle, :id => simply_another_user
}.should change { simply_another_user.reload.active }

# vs

user = Factory(:user, :active => true)
post :toggle, :id => user
User.find(user.id).active.should be_false

To me, the latter is easier to follow because it's more concise, more declarative (create a user with :active => true rather than setting an expectation that it's true before the event in the example), more truthful (in reality, the post does not touch the same object), and doesn't rely on idiosyncrasies of ActiveRecord (i.e. reload). But that's for me. If you think the former is more readable, than that's the form you should use because you're the one that's going to have to read it.

Owner

dchelimsky commented Feb 1, 2012

"readability" is subjective, and you're not comparing the entire example. Compare these:

user = Factory(:user)
user.active.should be_true
lambda {
  post :toggle, :id => simply_another_user
}.should change { simply_another_user.reload.active }

# vs

user = Factory(:user, :active => true)
post :toggle, :id => user
User.find(user.id).active.should be_false

To me, the latter is easier to follow because it's more concise, more declarative (create a user with :active => true rather than setting an expectation that it's true before the event in the example), more truthful (in reality, the post does not touch the same object), and doesn't rely on idiosyncrasies of ActiveRecord (i.e. reload). But that's for me. If you think the former is more readable, than that's the form you should use because you're the one that's going to have to read it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment