Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > Ruby > exercise in DRY

Reply
Thread Tools

exercise in DRY

 
 
Peter Ehrlich
Guest
Posts: n/a
 
      08-18-2009
I have some simple code for a thumbs up/thumbs down functionality.
Clicking "agree" sends an ajax count to increase the agree field by one,
clicking disagree increases the "disagree" count by one. The text
returned is both values with the updated one shown as "oldval <strong>
+1</strong>"

Is there any good way to make this pretty? The most I can think of is a
boolean argument specifying agree/disagree, but that doesn't really make
things any simpler :-/

Thanks!
--Peter


def agree
tweet = tweet.find(params[:id])
tweet.agree_count += 1
render :text => consensus_agree(tweet.agree_count,
tweet.disagree_count)
end

def disagree
tweet = tweet.find(params[:id])
tweet.disagree_count += 1
render :text => consensus_agree(tweet.agree_count,
tweet.disagree_count)
end

private
def consensus_agree(agree_count, disagree_count)
"<span style='color:green;' >#{agree_count -1} <strong>+
1</strong></span>
&ndash;
<span style='color:red;'>#{disagree_count}</span>"
end

def consensus_disagree(agree_count, disagree_count)
"<span class='agree'>#{agree_count}</span>
&ndash;
<span class='disagree'>#{disagree_count -1} <strong>+
1</strong></span>"
end
--
Posted via http://www.ruby-forum.com/.

 
Reply With Quote
 
 
 
 
Fabian Streitel
Guest
Posts: n/a
 
      08-18-2009
[Note: parts of this message were removed to make it a legal post.]

How 'bout that:

class Tweet
def agree
agree_count += 1
end

def disagree
disagree_count += 1
end
end

# pass this method either :agree or :disagree
def agree_or_not( method )
tweet = tweet.find(params[:id])
tweet.send(method)
render :text => consensus(method, tweet.agree_count, tweet.disagree_count)
end

def consensus( method, agreed, disagreed )
if method == :agree
agreed = "#{agreed-1} <strong>+1</strong>"
else
disagreed = "#{disagreed-1} <strong>+1</strong>"
end

"<span class='agree'>#{agreed}</span> &ndash; <span
class='disagree'>#{disagreed}</span>"
end

 
Reply With Quote
 
 
 
 
Peter Ehrlich
Guest
Posts: n/a
 
      08-18-2009
This look pretty good! I have to set up a bunch of migrations before I
can test, so I'll just ask now; what do I do with the Tweet class?
(Still a bit of a newbie here!) I notice that you don't say self.agree
as the method name - this means that they're only added for the scope
the one request, right?

Thanks!
--Peter

Fabian Streitel wrote:
> How 'bout that:
>
> class Tweet
> def agree
> agree_count += 1
> end
>
> def disagree
> disagree_count += 1
> end
> end
>
> # pass this method either :agree or :disagree
> def agree_or_not( method )
> tweet = tweet.find(params[:id])
> tweet.send(method)
> render :text => consensus(method, tweet.agree_count,
> tweet.disagree_count)
> end
>
> def consensus( method, agreed, disagreed )
> if method == :agree
> agreed = "#{agreed-1} <strong>+1</strong>"
> else
> disagreed = "#{disagreed-1} <strong>+1</strong>"
> end
>
> "<span class='agree'>#{agreed}</span> &ndash; <span
> class='disagree'>#{disagreed}</span>"
> end


--
Posted via http://www.ruby-forum.com/.

 
Reply With Quote
 
Fabian Streitel
Guest
Posts: n/a
 
      08-18-2009
[Note: parts of this message were removed to make it a legal post.]

2009/8/18 Peter Ehrlich <(E-Mail Removed)>

> This look pretty good! I have to set up a bunch of migrations before I
> can test, so I'll just ask now; what do I do with the Tweet class?
> (Still a bit of a newbie here!) I notice that you don't say self.agree
> as the method name - this means that they're only added for the scope
> the one request, right?
>


nope, they are normal instance methods. I assume you are using something
like DataMapper or ActiveRecord to handle your database access.

So in your model class Tweet (I assume that's what it's called) you add
these
two methods. Then they can be called on all the Tweet instances.

BTW, don't you have to call tweet.save or similar some time in your code?

Greetz!

 
Reply With Quote
 
Peter Ehrlich
Guest
Posts: n/a
 
      08-18-2009
Alright awesome, thanks for the help! (And yes I suppose I should throw
a save in there

--Peter

Fabian Streitel wrote:
>
> nope, they are normal instance methods. I assume you are using something
> like DataMapper or ActiveRecord to handle your database access.
>
> So in your model class Tweet (I assume that's what it's called) you add
> these
> two methods. Then they can be called on all the Tweet instances.
>
> BTW, don't you have to call tweet.save or similar some time in your
> code?
>
> Greetz!


--
Posted via http://www.ruby-forum.com/.

 
Reply With Quote
 
Kaspar Schiess
Guest
Posts: n/a
 
      08-19-2009
How about: (assumes rails and haml)

before_filter :find_tweet, nly => [:agree, :disagree_count]

def find_tweet
@tweet = tweet.find(params[:id])
end

def agree
@tweet.agree_count += 1
render artial => 'consensus_agree'
end

def disagree
@tweet.disagree_count += 1
render artial => 'consensus_agree'
end

--------- _consensus_agree.haml ---------------

%span{:style => 'color: green'}= @tweet.agree_count - 1
%span{:style => 'color: red'}= @tweet.disagree_count

-----------------------------------------------

Also: You might have forgotten a @tweet.save there. And.. you really
should be using classes, not styles. </nitpick>

regards,
kaspar


 
Reply With Quote
 
Fabian Streitel
Guest
Posts: n/a
 
      08-19-2009
[Note: parts of this message were removed to make it a legal post.]

that's not quite correct

How about: (assumes rails and haml)


> before_filter :find_tweet, nly => [:agree, :disagree_count]



(nice one, I totally forgot the filters... )

>
> def find_tweet
> @tweet = tweet.find(params[:id])
> end
>
> def agree
> @tweet.agree_count += 1
> render artial => 'consensus_agree'
> end
>
> def disagree
> @tweet.disagree_count += 1
> render artial => 'consensus_agree'



this has to be
render artial => 'consensus_disagree'
of course

end
>
> --------- _consensus_agree.haml ---------------
>
> %span{:style => 'color: green'}= @tweet.agree_count - 1
> %span{:style => 'color: red'}= @tweet.disagree_count
>
> -----------------------------------------------



so you'd have to have a second haml file for disagree.
Greetz

 
Reply With Quote
 
Peter Ehrlich
Guest
Posts: n/a
 
      08-19-2009
Alright, thanks to both of you, its working beautifully (and I've
learned a couple new things..)

A mistake in my css has made me think that class names were not working
ajax - but thats remedied now.

Cheers,
--Peter

Fabian Streitel wrote:
> that's not quite correct
>
> How about: (assumes rails and haml)
>
>
>> before_filter :find_tweet, nly => [:agree, :disagree_count]

>
>
> (nice one, I totally forgot the filters... )
>
>> def disagree
>> @tweet.disagree_count += 1
>> render artial => 'consensus_agree'

>
>
> this has to be
> render artial => 'consensus_disagree'
> of course
>
> end
>>
>> --------- _consensus_agree.haml ---------------
>>
>> %span{:style => 'color: green'}= @tweet.agree_count - 1
>> %span{:style => 'color: red'}= @tweet.disagree_count
>>
>> -----------------------------------------------

>
>
> so you'd have to have a second haml file for disagree.
> Greetz


--
Posted via http://www.ruby-forum.com/.

 
Reply With Quote
 
Peter Ehrlich
Guest
Posts: n/a
 
      08-19-2009
So for anyone curious, here's how I wound up doing it:
(no haml just because I'm too busy learning rails and ruby already

It probably could be made more dry, but this is pretty good and has the
benefit of being written. I haven't included the controller, because
its a bit more complex, dealing with different databases of anonymous
and non-anonymous users..

==== _agree_field.erb ====
<% #logger.debug "view agree: " + tweet.votedOn.to_s -%>
<% if tweet.votedOn != nil -%>
<% if tweet.votedOn == 1 #agree %>
<% agree = "#{tweet.agree_count - 1}<strong> + 1</strong>" %>
<% disagree = tweet.disagree_count.to_s %>
<% elsif tweet.votedOn == 0 #disagree%>
<% disagree = "#{tweet.disagree_count - 1}<strong> + 1</strong>" %>
<% agree = tweet.agree_count.to_s -%>
<% elsif tweet.votedOn == 2 #neutral -%>
<% agree = tweet.agree_count.to_s -%>
<% disagree = tweet.disagree_count.to_s %>
<% end %>
<span class='agree'><%= agree %> </span>
&ndash;
<span class='disagree'><%= disagree %></span>
<% else -%>

<span class="agree">
<%= link_to_remote('Agree', :update => "#{tweet.dom_id}_c", :url
=> "/tweets/agree/#{tweet.id}?agree=true") %>
</span>
<%= link_to_remote('&ndash;', :update => "#{tweet.dom_id}_c", :url
=> "/tweets/agree/#{tweet.id}")%>
<span class="disagree">
<%= link_to_remote('Disagree', :update => "#{tweet.dom_id}_c",
:url => "/tweets/agree/#{tweet.id}?agree=false")%>
</span>

<% end -%>


Cheers,
--Peter
--
Posted via http://www.ruby-forum.com/.

 
Reply With Quote
 
Josh Cheek
Guest
Posts: n/a
 
      08-20-2009
[Note: parts of this message were removed to make it a legal post.]

Hi, Peter,
If you have that many <% ... %> all in a row, and are placing that much
logic into your view, then you are probably ripe for a helper In the file
app/helpers/tweet_helper.rb you can declare a method like

def agree_and_disagree(tweet)
if tweet.votedOn == 1 #agree
a"#{tweet.agree_count - 1}<strong> + 1</strong>"
disagree = tweet.disagree_count.to_s
elsif tweet.votedOn == 0 #disagree
disagree = "#{tweet.disagree_count - 1}<strong> + 1</strong>"
agree = tweet.agree_count.to_s
elsif tweet.votedOn == 2 #neutral
agree = tweet.agree_count.to_s
disagree = tweet.disagree_count.to_s
end
[agree , disagree]
end

This can be refactored a bit, into

def agree_and_disagree(tweet)
if tweet.votedOn == 1 #agree
["#{tweet.agree_count - 1}<strong> + 1</strong>" ,
tweet.disagree_count.to_s]
elsif tweet.votedOn == 0 #disagree
["#{tweet.disagree_count - 1}<strong> + 1</strong>" ,
tweet.agree_count.to_s]
elsif tweet.votedOn == 2 #neutral
[tweet.agree_count.to_s , tweet.disagree_count.to_s]
end
end


Which cleans up your view code, you can then assign the values with
something like
<% if tweet.votedOn != nil -%>
<% agree , disagree = agree_and_disagree(tweet) %>
<span class='agree'><%= agree %> </span>
&ndash;
<span class='disagree'><%= disagree %></span>
<% else -%>
...



Also, to make your code more readable and intuitive, you can take
if tweet.votedOn == 1 #agree
elsif tweet.votedOn == 0 #disagree
elsif tweet.votedOn == 2 #neutral

And create methods for these in your model (or the tweet's class, however
you have it defined). Something like

class Tweet < ActiveRecord::Base
...
def agree?() votedOn == 1 end
def disagree?() votedOn == 0 end
def neutral?() votedOn == 2 end
end

Then your helper code can be just
if tweet.agree?
elsif tweet.disagree?
elsif tweet.neutral?

You can, of course, name these whatever you think makes them the most
readable and is the most memorable. This means that you don't have to
remember that they agree if votedOn is equal to some internally defined
value, it helps keep those values inside the class, and just provides
intuitive, self-descriptive methods as an interface. After you get that
class made, you should ideally never have to know that 1 means agree, and so
on. That can all be handled internally, making your class easier to work
with. (If you are working with this class a lot, you'll thank yourself later
for establishing a nice set of methods to encapsulate internal logic).



Also, FWIW, if you don't like passing arguments in the URL, you can define a
route like
/tweets/agree/:id/agree
/tweets/agree/:id/disagree
By declaring them as members in your routes and having methods in your
controller for them to invoke. If you are interested in that, here is a
wonderful guide on routing
http://guides.rubyonrails.org/routin...estful-actions




On Wed, Aug 19, 2009 at 6:17 PM, Peter Ehrlich <(E-Mail Removed)>wrote:

> So for anyone curious, here's how I wound up doing it:
> (no haml just because I'm too busy learning rails and ruby already
>
> It probably could be made more dry, but this is pretty good and has the
> benefit of being written. I haven't included the controller, because
> its a bit more complex, dealing with different databases of anonymous
> and non-anonymous users..
>
> ==== _agree_field.erb ====
> <% #logger.debug "view agree: " + tweet.votedOn.to_s -%>
> <% if tweet.votedOn != nil -%>
> <% if tweet.votedOn == 1 #agree %>
> <% agree = "#{tweet.agree_count - 1}<strong> + 1</strong>" %>
> <% disagree = tweet.disagree_count.to_s %>
> <% elsif tweet.votedOn == 0 #disagree%>
> <% disagree = "#{tweet.disagree_count - 1}<strong> + 1</strong>" %>
> <% agree = tweet.agree_count.to_s -%>
> <% elsif tweet.votedOn == 2 #neutral -%>
> <% agree = tweet.agree_count.to_s -%>
> <% disagree = tweet.disagree_count.to_s %>
> <% end %>
> <span class='agree'><%= agree %> </span>
> &ndash;
> <span class='disagree'><%= disagree %></span>
> <% else -%>
>
> <span class="agree">
> <%= link_to_remote('Agree', :update => "#{tweet.dom_id}_c", :url
> => "/tweets/agree/#{tweet.id}?agree=true") %>
> </span>
> <%= link_to_remote('&ndash;', :update => "#{tweet.dom_id}_c", :url
> => "/tweets/agree/#{tweet.id}")%>
> <span class="disagree">
> <%= link_to_remote('Disagree', :update => "#{tweet.dom_id}_c",
> :url => "/tweets/agree/#{tweet.id}?agree=false")%>
> </span>
>
> <% end -%>
>
>
> Cheers,
> --Peter
> --
> Posted via http://www.ruby-forum.com/.
>
>


 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Some dry humour =?ISO-8859-1?Q?R=F4g=EAr?= Computer Support 1 12-15-2004 08:26 PM
Dry mounting HANSTEIS Digital Photography 3 01-30-2004 11:18 PM
Dry mounting ink jet prints Birk Binnard Digital Photography 4 11-12-2003 12:54 PM
How to dry mount ink jet prints Birk Binnard Digital Photography 9 10-25-2003 06:12 PM
Inkjet -- time to dry? Lawrence King Digital Photography 3 08-05-2003 09:19 PM



Advertisments