I wanted to add a convenience method on my User class to see if he was already signed up for a tournament. Here''s my spec context "A User signed up for one tournament" do setup do @t1 = Tournament.new @t1.save false @t2 = Tournament.new @t2.save false @user = User.new @user.save false @user.registrations << Registration.new(:tournament => @t1) end specify "should be signed up for that tournament" do @user.should_be_signed_up_for @t1 end specify "should not be signed up for another tournament" do @user.should_not_be_signed_up_for @t2 end end class User < ActiveRecord::Base has_many :registrations def signed_up_for?(tournament) !registrations.find_by_tournament_id(tournament).nil? end end Are there any potential improvements, or is that the best way to spec it? Pat
On 1/1/07, Pat Maddox <pergesu at gmail.com> wrote:> I wanted to add a convenience method on my User class to see if he was > already signed up for a tournament. Here''s my spec > > context "A User signed up for one tournament" do > setup do > @t1 = Tournament.new > @t1.save false > @t2 = Tournament.new > @t2.save false > @user = User.new > @user.save false > @user.registrations << Registration.new(:tournament => @t1) > end > > specify "should be signed up for that tournament" do > @user.should_be_signed_up_for @t1 > end > > specify "should not be signed up for another tournament" do > @user.should_not_be_signed_up_for @t2 > end > end > > class User < ActiveRecord::Base > has_many :registrations > > def signed_up_for?(tournament) > !registrations.find_by_tournament_id(tournament).nil? > end > end > > Are there any potential improvements, or is that the best way to spec it?Couple of thoughts. Creating new unvalidated models seems to be something useful, so... module ModelSpecUtils def unvalidated_model(klass) model = klass.new model.save(false) model end end Also, this line: @user.registrations << Registration.new(:tournament => @t1) is breaking encapsulation a bit. Given that and the utility module above, I might end up w/ something like this: context "A User signed up for one tournament" do include ModelSpecUtils setup do @tournament1 = unvalidated_model(Tournament) @tournament2 = unvalidated_model(Tournament) @user = unvalidated_model(User) @user.register_for(@tournament1) end specify "should be signed up for that tournament" do @user.should_be_signed_up_for @tournament1 end specify "should not be signed up for another tournament" do @user.should_not_be_signed_up_for @tournament2 end end David> > Pat > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >
On 1/1/07, David Chelimsky <dchelimsky at gmail.com> wrote:> On 1/1/07, Pat Maddox <pergesu at gmail.com> wrote: > > I wanted to add a convenience method on my User class to see if he was > > already signed up for a tournament. Here''s my spec > > > > context "A User signed up for one tournament" do > > setup do > > @t1 = Tournament.new > > @t1.save false > > @t2 = Tournament.new > > @t2.save false > > @user = User.new > > @user.save false > > @user.registrations << Registration.new(:tournament => @t1) > > end > > > > specify "should be signed up for that tournament" do > > @user.should_be_signed_up_for @t1 > > end > > > > specify "should not be signed up for another tournament" do > > @user.should_not_be_signed_up_for @t2 > > end > > end > > > > class User < ActiveRecord::Base > > has_many :registrations > > > > def signed_up_for?(tournament) > > !registrations.find_by_tournament_id(tournament).nil? > > end > > end > > > > Are there any potential improvements, or is that the best way to spec it? > > Couple of thoughts. Creating new unvalidated models seems to be > something useful, so... > > module ModelSpecUtils > def unvalidated_model(klass) > model = klass.new > model.save(false) > model > end > end > > Also, this line: > > @user.registrations << Registration.new(:tournament => @t1) > > is breaking encapsulation a bit. Given that and the utility module > above, I might end up w/ something like this: > > context "A User signed up for one tournament" do > include ModelSpecUtils > > setup do > @tournament1 = unvalidated_model(Tournament) > @tournament2 = unvalidated_model(Tournament) > @user = unvalidated_model(User) > @user.register_for(@tournament1) > end > > specify "should be signed up for that tournament" do > @user.should_be_signed_up_for @tournament1 > end > > specify "should not be signed up for another tournament" do > @user.should_not_be_signed_up_for @tournament2 > end > end > > DavidMy original spec is bad, because it doesn''t show how the Registration model is being used. Right now in my controller I''m just doing @registration = Registration.new :tournament => @tournament, :user => current_user and then saving it. It''s probably not good then that I''m doing user.registrations<< in the spec. I know that breaks encapsulation, but I''m not doing it in my app. Either way though the specs should match the use. This brings up a more interesting question (to me at least. It could be pointless to some). I''ve got User and Tournament models, and have the Registration relationship. What''s the best way to create the Registration? Creating User#register_for has a couple upsides 1. Clients only need to know about two classes, user and tournament. 2. It reads really well. this_user.register_for(some_tourney) is very clear a. It''s "opinionated." Having an explicit method at least promotes a single creation mechanism (it gets a letter because I''m not sure if it''s that important) #2 is the biggest benefit, in my opinion, and it should be obvious why it''s good. #1 though is interesting to me, because while you want to couple clients to as few classes as possible, here you lose/diminish the value of a richer model. When you create a relationship model like this, presumably there are some interesting attributes other than the simple relationship. In my case, there''s nothing else interesting (yet). I use the join model because it''s more flexible and no more conceptually difficult than using HABTM. I prefer the explicit relationship. But maybe I need to signify that the relationship doesn''t have interesting attributes by abstracting it away behind the #register_for method. So here''s my hypothesis: when there are no interesting attributes to the relationship, create a simple method that hides the join class. When there are interesting attributes, create the relationship object explicitly. Of course you can never stick to simple rules like that, but hopefully it''s enough for you to give me another OOP lesson :) Pat
On 1/4/07, Pat Maddox <pergesu at gmail.com> wrote:> On 1/1/07, David Chelimsky <dchelimsky at gmail.com> wrote: > > On 1/1/07, Pat Maddox <pergesu at gmail.com> wrote: > > > I wanted to add a convenience method on my User class to see if he was > > > already signed up for a tournament. Here''s my spec > > > > > > context "A User signed up for one tournament" do > > > setup do > > > @t1 = Tournament.new > > > @t1.save false > > > @t2 = Tournament.new > > > @t2.save false > > > @user = User.new > > > @user.save false > > > @user.registrations << Registration.new(:tournament => @t1) > > > end > > > > > > specify "should be signed up for that tournament" do > > > @user.should_be_signed_up_for @t1 > > > end > > > > > > specify "should not be signed up for another tournament" do > > > @user.should_not_be_signed_up_for @t2 > > > end > > > end > > > > > > class User < ActiveRecord::Base > > > has_many :registrations > > > > > > def signed_up_for?(tournament) > > > !registrations.find_by_tournament_id(tournament).nil? > > > end > > > end > > > > > > Are there any potential improvements, or is that the best way to spec it? > > > > Couple of thoughts. Creating new unvalidated models seems to be > > something useful, so... > > > > module ModelSpecUtils > > def unvalidated_model(klass) > > model = klass.new > > model.save(false) > > model > > end > > end > > > > Also, this line: > > > > @user.registrations << Registration.new(:tournament => @t1) > > > > is breaking encapsulation a bit. Given that and the utility module > > above, I might end up w/ something like this: > > > > context "A User signed up for one tournament" do > > include ModelSpecUtils > > > > setup do > > @tournament1 = unvalidated_model(Tournament) > > @tournament2 = unvalidated_model(Tournament) > > @user = unvalidated_model(User) > > @user.register_for(@tournament1) > > end > > > > specify "should be signed up for that tournament" do > > @user.should_be_signed_up_for @tournament1 > > end > > > > specify "should not be signed up for another tournament" do > > @user.should_not_be_signed_up_for @tournament2 > > end > > end > > > > David > > My original spec is bad, because it doesn''t show how the Registration > model is being used. Right now in my controller I''m just doing > @registration = Registration.new :tournament => @tournament, :user => > current_user > > and then saving it. It''s probably not good then that I''m doing > user.registrations<< in the spec. I know that breaks encapsulation, > but I''m not doing it in my app. Either way though the specs should > match the use. > > This brings up a more interesting question (to me at least. It could > be pointless to some). I''ve got User and Tournament models, and have > the Registration relationship. What''s the best way to create the > Registration? > > Creating User#register_for has a couple upsides > 1. Clients only need to know about two classes, user and tournament. > 2. It reads really well. this_user.register_for(some_tourney) is very clear > a. It''s "opinionated." Having an explicit method at least promotes a > single creation mechanism (it gets a letter because I''m not sure if > it''s that important) > > #2 is the biggest benefit, in my opinion, and it should be obvious why > it''s good. > > #1 though is interesting to me, because while you want to couple > clients to as few classes as possible, here you lose/diminish the > value of a richer model. When you create a relationship model like > this, presumably there are some interesting attributes other than the > simple relationship. > > In my case, there''s nothing else interesting (yet). I use the join > model because it''s more flexible and no more conceptually difficult > than using HABTM. I prefer the explicit relationship. But maybe I > need to signify that the relationship doesn''t have interesting > attributes by abstracting it away behind the #register_for method. > > So here''s my hypothesis: when there are no interesting attributes to > the relationship, create a simple method that hides the join class. > When there are interesting attributes, create the relationship object > explicitly.That probably makes sense a lot of the time, though I can imagine a case where there are interesting attributes in the join but it *still* makes things most clear to do it through the user. Here''s one: @guest = Guest.new(:smoker => false) @hotel = Hotel.new @hotel.should_receive(:reserve_room).with(:start_date => Time.new, :nights => 3, :smoking_preference => "non-smoking'') @guest.reserve_room_at(@hotel) In this case, the guest is responsible to let the hotel know that she''s interested in a non-smoking room rather than the controller being responsible for asking the guest her preference and passing that on to the hotel.> > Of course you can never stick to simple rules like that, but hopefully > it''s enough for you to give me another OOP lesson :)You''re too kind.> > PatDavid> _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >