As I get deeper into my app, I find myself writing a lot of "if" statements. If a user is logged in I want to highlight her name on various postings and comments. If a user is logged in, I want their profile page to say "Your last 10 comments" versus "username''s last 10 comments". A partial is used to display a post. If a post is displayed on a list screen, link the post title to a post_detail page. If a post is displayed on a post_detail page, don''t link the post title to anything. I just need a nudge in the right direction. Should I simply be moving all this "if" code to helper classes? Or am I missing a pretty standard OO or RoR concept here that would make my code much more solid and less, well, "iffy". --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
What I like to do is use the "ternary" operator, which works like this: <%= (session[:user].id == params[:id])? "Your" : session[:user].username + "''s" %> Last 10 Comments The statement in the ( ) is evaluated. If true, the value to the left of the : is used. If false, the value to the right. Works wonders for shortening up code. And no, I don''t think you need to reformat your code. But you could. If there are formatting tasks you repeat a lot, put it into a helper. But if you just do it once, leave it alone. bjhess wrote:> As I get deeper into my app, I find myself writing a lot of "if" > statements. > > If a user is logged in I want to highlight her name on various postings > and comments. > > If a user is logged in, I want their profile page to say "Your last 10 > comments" versus "username''s last 10 comments". > > A partial is used to display a post. If a post is displayed on a list > screen, link the post title to a post_detail page. If a post is > displayed on a post_detail page, don''t link the post title to anything. > > I just need a nudge in the right direction. Should I simply be moving > all this "if" code to helper classes? Or am I missing a pretty > standard OO or RoR concept here that would make my code much more solid > and less, well, "iffy". > > > > > >--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Going throw college learning C and C++ I definitely got hooked on the ternary operator. Good point. I have a lot of instances of "do this if". So things like: <%= "<span class="highlight">" if session[:user_id] == @post.user_id %> Here''s the other wrinkle. If a user is logged in, I''m placing a highlight span around his posting title. This means I''m doing an if check above and below the title text. Just adding information to my original post. Jason Norris wrote:> What I like to do is use the "ternary" operator, which works like this: > > <%= (session[:user].id == params[:id])? "Your" : session[:user].username > + "''s" %> Last 10 Comments > > The statement in the ( ) is evaluated. If true, the value to the left of > the : is used. If false, the value to the right. > Works wonders for shortening up code. And no, I don''t think you need to > reformat your code. But you could. > If there are formatting tasks you repeat a lot, put it into a helper. > But if you just do it once, leave it alone. > > bjhess wrote: > > As I get deeper into my app, I find myself writing a lot of "if" > > statements. > > > > If a user is logged in I want to highlight her name on various postings > > and comments. > > > > If a user is logged in, I want their profile page to say "Your last 10 > > comments" versus "username''s last 10 comments". > > > > A partial is used to display a post. If a post is displayed on a list > > screen, link the post title to a post_detail page. If a post is > > displayed on a post_detail page, don''t link the post title to anything. > > > > I just need a nudge in the right direction. Should I simply be moving > > all this "if" code to helper classes? Or am I missing a pretty > > standard OO or RoR concept here that would make my code much more solid > > and less, well, "iffy". > > > > > > > > > > >--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
bjhess wrote:> As I get deeper into my app, I find myself writing a lot of "if" > statements.Open an old, boring book from ~10 years ago called /Design Patterns/. Write tests for every single effect of your if statements. If you duplicate any if statement, refactor your code to create objects with polymorphic methods. Run all the tests between each small code change. Share each new object between the places where the if statements duplicated. This is the heart of OO.> If a user is logged in, I want their profile page to say "Your last 10 > comments" versus "username''s last 10 comments".Start by "hiding" those if statements. Put them inside method calls that return either Your or ''usernames''s. Then move the if statement itself out to a function that decides - once and only once - which kind of relationship your application has with the user. if current_user == @user then FamiliarInterface.new else SociableInterface.new end. Pass that object around, and put inside it every if statement that dealt with familiarity with the user. You may eventually arrive at the Decorator Pattern. BTW one major problem with Ruby is you won''t learn many of the classical design patterns, such as Prototype or Adapter. Ruby makes them way too easy. /Design Patterns/ was written with C++ and Smalltalk examples, so you may need to switch to a harder language to learn them. ;-)> A partial is used to display a post. If a post is displayed on a list > screen, link the post title to a post_detail page. If a post is > displayed on a post_detail page, don''t link the post title to anything.In this case, pass a :local into the partial that writes the link. The caller knows which object to pass in. The caller could even decide to pass in a block closure. Or it could chose from predefined and reusable objects. Look up "Construction Encapsulation" - another critical technique that Ruby makes too easy.> I just need a nudge in the right direction. Should I simply be moving > all this "if" code to helper classes? Or am I missing a pretty > standard OO or RoR concept here that would make my code much more solid > and less, well, "iffy".Yes. The heart of OO, and the reason OO is a powerful design technique, is that each time you replace a conditional with polymorphism, you have a new object that can now replace other conditionals The great thing about Rails is it already did a lot of this OO polymorphism for you. You no longer have to claw your way through a maze of if Database if Controller if View nonsense. These kids these days probably don''t remember classical Perl CGI website development! Back in my day, we had to bless our hashes!! The power of Rails has lead you to create a complex beast of your own, hosted within it. So you now need to apply your own abstractions, to keep your own layer of the design clean and flexible. If you don''t clean up your design (and if you don''t write unit tests) then each time you add a new feature, you must balance its new if statements against all your old ones, including ones you might forget about. This decay will cause bugs, and the design quality will go down. Then, fixing bugs will cause more bugs. So the time to learn OO and apply it here is as early as possible, before the design decays! -- Phlip --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Philip, thanks for the reply.
I was thinking about highlighting and threw together this design. In
the end, I don''t know that it is useful. But it was a worthwhile
activity. This is almost exactly like Design Patterns'' Decorator
pattern. I''m wondering if there is a way to bake the user comparisons
into this somehow.
An abstract TextComponent class with a TextComponent(text) constructor
and a text attribute. The constructor stores the text parameter in the
text attribute. There is also a render method that would simply return
the text attribute.
A ConcreteTextComponent class that extends the abstract.
A HighlightDecorator that extends the TextComponent abstract class. It
would have a HighlightDecorator(TextComponent) constructor. It would
have a text_component attribute. It''s render method would look like:
def render
"<span
class=''highlight''>#{text_component.render}</span>"
end
In the ApplicationHelper class there would be a
print_text(TextComponent) method. It would simply do
text_component.render.
In this way all highlighting in the app would be achieved with the
following call:
print_text(HighlightDecorator.new(ConcreteTextComponent.new("text")))
Not sure if that is an improvement over the inline highlight spanning.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---
bjhess wrote:> print_text(HighlightDecorator.new(ConcreteTextComponent.new("text"))) > > Not sure if that is an improvement over the inline highlight spanning.That is Java-style Ruby. Can''t you do a trick with fewer variables, and a block closure? -- Phlip --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
On Nov 17, 2006, at 2:23 PM, Phlip wrote:> bjhess wrote: > >> print_text(HighlightDecorator.new(ConcreteTextComponent.new("text"))) >> >> Not sure if that is an improvement over the inline highlight >> spanning. > > That is Java-style Ruby. Can''t you do a trick with fewer variables, > and > a block closure? > > -- > PhlipI started writing this response earlier, but it makes more sense here anyway ;-) You can always choose to hide some of those ''if'' statements in some helper methods: # In a helper module, you could put: def user_possessive(user_id) if session[:user].id == user_id "Your" else session[:user].username + "''s" end end # In your view: <%= user_possesive(params[:id]) %> Last 10 Comments # For the optional span tags, perhaps something like this in a helper: def highlight_if(condition) if condition content_tag(:span, yield, :class => ''highlight'') else yield end end # which in a view would be used like: <%= highlight_if(session[:user_id] == @post.user_id) { @post.title } %> Sorry I didn''t Decorate and Componentize it as much 8^) -Rob Rob Biedenharn http://agileconsultingllc.com Rob-xa9cJyRlE0mWcWVYNo9pwxS2lgjeYSpx@public.gmane.org --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Rob Biedenharn wrote:> Sorry I didn''t Decorate and Componentize it as much 8^)Just so long as you didn''t OCP it. :-] -- Phlip --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Phlip wrote:> bjhess wrote: > > > print_text(HighlightDecorator.new(ConcreteTextComponent.new("text"))) > > > > Not sure if that is an improvement over the inline highlight spanning. > > That is Java-style Ruby.There''s definitely a reason for that. I''m a Java-convert trying to make my way in a Ruby world. :) --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
bjhess wrote:> There''s definitely a reason for that. I''m a Java-convert trying to > make my way in a Ruby world. :)Heck - I started with C++ style Ruby. Imagine the horror! Just for you, I added a simple feature to my "how to Chat" project. I made my new feature extra-complex, to show what we meant by the Decorator pattern in Ruby. I''m not sure if this is true Decorator - that''s the great thing about Ruby; it makes abstractions so easy that you don''t need to know if they are actually design patterns or not. But at least my pattern ... decorates my chat panel. This calls the partial. Stare at the last =>: <%= render :partial => ''chat/index'', :locals => { :chat_style => { :panel_id => 2, :panel_height => ''10ex'', :get_background_color => proc { ''#eeeeee'' } } } That sets the :get_background_color parameter to a block. The block simply returns a string, but it _could_ have returned one branch of your ''if'' statement. Maybe we have complex caller-specific logic to set the color. If this logic belongs to the caller, then we should not taint the partial with knowledge about the caller. The caller can put its own variables into the proc{}, even local ones. The partial system turns :chat_style into a local variable (per :locals). Then we can read its parameters out of it: panel_id = chat_style[:panel_id] panel_height = chat_style[:panel_height] || ''30ex'' get_background_color = chat_style[:get_background_color] || proc { ''#ffffee'' } (There may be other ways to pass locals and then provide defaults...) I provide a default for :get_background_color, of a different string. Again, that proc could have contained complex logic to generate the background color. Consider this the ''else'' branch of your ''if'' statement. Now we finally put our lowly background color generating proc to good use: <div id=''panel_<%= panel_id %>_chat_history'' style=''height: <%= panel_height %>; font-size: 90%; border-bottom: #eeeeee solid 1px; overflow: auto; background-color: <%= get_background_color.call %>'' > The result is our DIV style string doesn''t contain an ''if'' _or_ an ''else''. It only contains a call to a polymorphically defined function. The target functions are defined up in the modules best able to direct their contents. -- Phlip --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Thanks for all the replies. Excellent information here! Another fairly complicated scenario I have going. On the post detail screen, numerous things can display three different ways depending on if the user is the original author, not the original author, or if no user is logged in. For instance, at the bottom of the post comments, there is a form to write a comment. It will only appear if there is a user logged in and said user is not the post''s author. The form will not display if the logged in user is the post''s author. If there is no logged in user? A "Please register to comment" message will appear. I''ve refactored this once into a helper method so that my view now only has a call to "comment_entry(@user, @post)". At the least, this gives me some ability to unit test the process. (Side note: To keep track of the user''s author status, I''m setting an "is_author" attribute on the user instance. What feels wrong about this is that I''m directly setting this attribute in a controller at the times that I care about author status.) --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---