Rails - Apr 2006 - Best way to propogate model rules to controller?

Hey everybody,

I''ve got a model that represents kind of a turn-based games.  Certain
actions can only be made unders certain conditions.  For simplicity,
we''ll say that the only condition for a particular action is that the
weekday be Tuesday.

def add_vote
  raise ''You cannot vote today'' unless Date.today.wday == 2
  ...
end

In my controller I want to show a link, but only if the user is
allowed to vote.  I''m wondering the best way to do this.  I think the
best way is to make another method, which I can call from within both
the model and controller.

def allow_vote?
  Date.today.wday == 2
end

def add_vote
  raise ''You cannot vote today'' unless Date.today.wday == 2
  ...
end

Then in my view I can just do <% unless game.allow_vote? %> to hide
the link.  In the controller actions that handle voting, I just rescue
the exception and display an error if necessary.

Does that seem like a good approach, or is there something better?  I
need this as some rules vary from game to game, and are stored in the
db for each game.

Pat

On Mon, Apr 10, 2006 at 04:28:29AM -0600, Pat Maddox wrote:
} I''ve got a model that represents kind of a turn-based games.  Certain
} actions can only be made unders certain conditions.  For simplicity,
} we''ll say that the only condition for a particular action is that the
} weekday be Tuesday.
[...]

This is business logic, and is appropriate for inclusion in your model.

} In my controller I want to show a link, but only if the user is
} allowed to vote.  I''m wondering the best way to do this.  I think the
} best way is to make another method, which I can call from within both
} the model and controller.
} 
} def allow_vote?
}   Date.today.wday == 2
} end

The above method belongs in your model. If it depended on which user was
trying to vote, the user should be passed as an argument to the method.

} def add_vote
}   raise ''You cannot vote today'' unless Date.today.wday == 2
}   ...
} end

This method should probably be an action in your controller, and it should
almost certainly give a nicer response than an exception. It should also be
implemented more like (pseudocode):

def add_vote
  #param stuff...
  if @game.allow_vote?
    @game.votes.create(...)
    render :action => ''voted''
  else
    render :action => ''rejectvote''
  end
end

} Then in my view I can just do <% unless game.allow_vote? %> to hide
} the link.

Yes.

} In the controller actions that handle voting, I just rescue the exception
} and display an error if necessary.
} 
} Does that seem like a good approach, or is there something better?  I
} need this as some rules vary from game to game, and are stored in the
} db for each game.

There is no need for an exception at all, just a method on your model that
determines whether voting is allowed. If this determination depends on data
in the database, the allow_vote? method will be somewhat more complex. If
it depends on the game type, meaning it make sense to implement the
allow_vote? method separately for each game type, you probably want to look
into Single Table Inheritance (STI) and implement allow_vote? separately in
each subclass.

} Pat
--Greg

Sorry, I wasn''t clear as to where the methods would go :)

allow_vote? and add_vote would both be in the model.  add_vote would
throw an exception if the tries to submit a vote when he shouldn''t. 
This shouldn''t even happen, so it''s exceptional.

In my view, I would use the model''s allow_vote? method to see if I
should even display a link.  In the controller that collects the vote,
it will just proxy it on to the model''s submit_vote method, displaying
an error if an exception is thrown.  The only way an exception would
be thrown is if the user were to directly access the submit_vote
action when he wasn''t supposed to (by creating a form of his own or
something like that).

Does that make more sense?  Basically the model itself should know
when votes are allowed, and the view uses these allow_xx? helpers to
determine what links/partials to render.

Pat



On 4/10/06, Gregory Seidman <gsslist+ror@anthropohedron.net>
wrote:
> On Mon, Apr 10, 2006 at 04:28:29AM -0600, Pat Maddox wrote:
> } I''ve got a model that represents kind of a turn-based games. 
Certain
> } actions can only be made unders certain conditions.  For simplicity,
> } we''ll say that the only condition for a particular action is
that the
> } weekday be Tuesday.
> [...]
>
> This is business logic, and is appropriate for inclusion in your model.
>
> } In my controller I want to show a link, but only if the user is
> } allowed to vote.  I''m wondering the best way to do this.  I
think the
> } best way is to make another method, which I can call from within both
> } the model and controller.
> }
> } def allow_vote?
> }   Date.today.wday == 2
> } end
>
> The above method belongs in your model. If it depended on which user was
> trying to vote, the user should be passed as an argument to the method.
>
> } def add_vote
> }   raise ''You cannot vote today'' unless Date.today.wday
== 2
> }   ...
> } end
>
> This method should probably be an action in your controller, and it should
> almost certainly give a nicer response than an exception. It should also be
> implemented more like (pseudocode):
>
> def add_vote
>   #param stuff...
>   if @game.allow_vote?
>     @game.votes.create(...)
>     render :action => ''voted''
>   else
>     render :action => ''rejectvote''
>   end
> end
>
> } Then in my view I can just do <% unless game.allow_vote? %> to hide
> } the link.
>
> Yes.
>
> } In the controller actions that handle voting, I just rescue the exception
> } and display an error if necessary.
> }
> } Does that seem like a good approach, or is there something better?  I
> } need this as some rules vary from game to game, and are stored in the
> } db for each game.
>
> There is no need for an exception at all, just a method on your model that
> determines whether voting is allowed. If this determination depends on data
> in the database, the allow_vote? method will be somewhat more complex. If
> it depends on the game type, meaning it make sense to implement the
> allow_vote? method separately for each game type, you probably want to look
> into Single Table Inheritance (STI) and implement allow_vote? separately in
> each subclass.
>
> } Pat
> --Greg
>
> _______________________________________________
> Rails mailing list
> Rails@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails
>

On Mon, Apr 10, 2006 at 06:19:45AM -0600, Pat Maddox wrote:
} Sorry, I wasn''t clear as to where the methods would go :)
} 
} allow_vote? and add_vote would both be in the model.  add_vote would
} throw an exception if the tries to submit a vote when he shouldn''t. 
} This shouldn''t even happen, so it''s exceptional.
} 
} In my view, I would use the model''s allow_vote? method to see if I
} should even display a link.  In the controller that collects the vote,
} it will just proxy it on to the model''s submit_vote method,
displaying
} an error if an exception is thrown.  The only way an exception would
} be thrown is if the user were to directly access the submit_vote
} action when he wasn''t supposed to (by creating a form of his own or
} something like that).

You should check allow_vote? in the voting controller action rather than
just calling a method that could throw an exception. And the add_vote
method should use the allow_vote? method in its test so the checking code
is maintained in a single method (DRY principle). Other than that, sure.

} Does that make more sense?  Basically the model itself should know
} when votes are allowed, and the view uses these allow_xx? helpers to
} determine what links/partials to render.

The view should use allow_xx? to conditionally display links/whatever, the
controller should use allow_xx? to handle malicious/mistaken actions, and
the model should use allow_xx? to sanity check its mutators. All the
checking happens in a single method, which means you only have to maintain
it in one place (DRY principle).

} Pat
--Greg

} On 4/10/06, Gregory Seidman <gsslist+ror@anthropohedron.net> wrote:
} > On Mon, Apr 10, 2006 at 04:28:29AM -0600, Pat Maddox wrote:
} > } I''ve got a model that represents kind of a turn-based games. 
Certain
} > } actions can only be made unders certain conditions.  For simplicity,
} > } we''ll say that the only condition for a particular action is
that the
} > } weekday be Tuesday.
} > [...]
} >
} > This is business logic, and is appropriate for inclusion in your model.
} >
} > } In my controller I want to show a link, but only if the user is
} > } allowed to vote.  I''m wondering the best way to do this.  I
think the
} > } best way is to make another method, which I can call from within both
} > } the model and controller.
} > }
} > } def allow_vote?
} > }   Date.today.wday == 2
} > } end
} >
} > The above method belongs in your model. If it depended on which user was
} > trying to vote, the user should be passed as an argument to the method.
} >
} > } def add_vote
} > }   raise ''You cannot vote today'' unless
Date.today.wday == 2
} > }   ...
} > } end
} >
} > This method should probably be an action in your controller, and it
should
} > almost certainly give a nicer response than an exception. It should also
be
} > implemented more like (pseudocode):
} >
} > def add_vote
} >   #param stuff...
} >   if @game.allow_vote?
} >     @game.votes.create(...)
} >     render :action => ''voted''
} >   else
} >     render :action => ''rejectvote''
} >   end
} > end
} >
} > } Then in my view I can just do <% unless game.allow_vote? %> to
hide
} > } the link.
} >
} > Yes.
} >
} > } In the controller actions that handle voting, I just rescue the
exception
} > } and display an error if necessary.
} > }
} > } Does that seem like a good approach, or is there something better?  I
} > } need this as some rules vary from game to game, and are stored in the
} > } db for each game.
} >
} > There is no need for an exception at all, just a method on your model
that
} > determines whether voting is allowed. If this determination depends on
data
} > in the database, the allow_vote? method will be somewhat more complex. If
} > it depends on the game type, meaning it make sense to implement the
} > allow_vote? method separately for each game type, you probably want to
look
} > into Single Table Inheritance (STI) and implement allow_vote? separately
in
} > each subclass.
} >
} > } Pat
} > --Greg
} >
} > _______________________________________________
} > Rails mailing list
} > Rails@lists.rubyonrails.org
} > http://lists.rubyonrails.org/mailman/listinfo/rails
} >
}

On 4/10/06, Gregory Seidman <gsslist+ror@anthropohedron.net>
wrote:
> On Mon, Apr 10, 2006 at 06:19:45AM -0600, Pat Maddox wrote:
> } Sorry, I wasn''t clear as to where the methods would go :)
> }
> } allow_vote? and add_vote would both be in the model.  add_vote would
> } throw an exception if the tries to submit a vote when he
shouldn''t.
> } This shouldn''t even happen, so it''s exceptional.
> }
> } In my view, I would use the model''s allow_vote? method to see if
I
> } should even display a link.  In the controller that collects the vote,
> } it will just proxy it on to the model''s submit_vote method,
displaying
> } an error if an exception is thrown.  The only way an exception would
> } be thrown is if the user were to directly access the submit_vote
> } action when he wasn''t supposed to (by creating a form of his own
or
> } something like that).
>
> You should check allow_vote? in the voting controller action rather than
> just calling a method that could throw an exception. And the add_vote
> method should use the allow_vote? method in its test so the checking code
> is maintained in a single method (DRY principle). Other than that, sure.
>
> } Does that make more sense?  Basically the model itself should know
> } when votes are allowed, and the view uses these allow_xx? helpers to
> } determine what links/partials to render.
>
> The view should use allow_xx? to conditionally display links/whatever, the
> controller should use allow_xx? to handle malicious/mistaken actions, and
> the model should use allow_xx? to sanity check its mutators. All the
> checking happens in a single method, which means you only have to maintain
> it in one place (DRY principle).
>
> } Pat
> --Greg
>
> } On 4/10/06, Gregory Seidman <gsslist+ror@anthropohedron.net> wrote:
> } > On Mon, Apr 10, 2006 at 04:28:29AM -0600, Pat Maddox wrote:
> } > } I''ve got a model that represents kind of a turn-based
games.  Certain
> } > } actions can only be made unders certain conditions.  For
simplicity,
> } > } we''ll say that the only condition for a particular action
is that the
> } > } weekday be Tuesday.
> } > [...]
> } >
> } > This is business logic, and is appropriate for inclusion in your
model.
> } >
> } > } In my controller I want to show a link, but only if the user is
> } > } allowed to vote.  I''m wondering the best way to do this. 
I think the
> } > } best way is to make another method, which I can call from within
both
> } > } the model and controller.
> } > }
> } > } def allow_vote?
> } > }   Date.today.wday == 2
> } > } end
> } >
> } > The above method belongs in your model. If it depended on which user
was
> } > trying to vote, the user should be passed as an argument to the
method.
> } >
> } > } def add_vote
> } > }   raise ''You cannot vote today'' unless
Date.today.wday == 2
> } > }   ...
> } > } end
> } >
> } > This method should probably be an action in your controller, and it
should
> } > almost certainly give a nicer response than an exception. It should
also be
> } > implemented more like (pseudocode):
> } >
> } > def add_vote
> } >   #param stuff...
> } >   if @game.allow_vote?
> } >     @game.votes.create(...)
> } >     render :action => ''voted''
> } >   else
> } >     render :action => ''rejectvote''
> } >   end
> } > end
> } >
> } > } Then in my view I can just do <% unless game.allow_vote? %>
to hide
> } > } the link.
> } >
> } > Yes.
> } >
> } > } In the controller actions that handle voting, I just rescue the
exception
> } > } and display an error if necessary.
> } > }
> } > } Does that seem like a good approach, or is there something better?
I
> } > } need this as some rules vary from game to game, and are stored in
the
> } > } db for each game.
> } >
> } > There is no need for an exception at all, just a method on your
model that
> } > determines whether voting is allowed. If this determination depends
on data
> } > in the database, the allow_vote? method will be somewhat more
complex. If
> } > it depends on the game type, meaning it make sense to implement the
> } > allow_vote? method separately for each game type, you probably want
to look
> } > into Single Table Inheritance (STI) and implement allow_vote?
separately in
> } > each subclass.
> } >
> } > } Pat
> } > --Greg
> } >
> } > _______________________________________________
> } > Rails mailing list
> } > Rails@lists.rubyonrails.org
> } > http://lists.rubyonrails.org/mailman/listinfo/rails
> } >
> }
> _______________________________________________
> Rails mailing list
> Rails@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails
>