Any help with refactory this ugly code?

11155 · January 20, 2012, 1:29pm

https://gist.github.com/1644085

#verifications/index.haml - if Ad.unverified_ads.size == 0 - else = link_to "Fast Verify", fast_verify_info_verification_path(Ad.unverified_ads.last), :method => :put = link_to "Normal Verify", verify_info_verification_path(ad), :method => :put

#verifications/verify_info.haml = link_to "Verify", verify_verification_path(@ad)

#verifications/fast_verify_info.haml = link_to "Verify", fast_verify_verification_path(Ad.unverified_ads.last), :method => :put

#verification_controller.rb def verify_info @ad = Ad.find(params[:id]) end

def fast_verify_info @ad = Ad.find(params[:id]) end

def fast_verify @ad = Ad.find(params[:id]) @ad.verify! if Ad.unverified_ads.size == 0 redirect_to verifications_path else redirect_to fast_verify_info_verification_path(Ad.unverified_ads.last) end end

def verify @ad = Ad.find(params[:id]) @ad.verify! if params[:fast] == true redirect_to Ad.unverified_ads.last else redirect_to verifications_path end end

I think there is too many actions, how to improve this?

Michael_Pavling · January 20, 2012, 2:22pm

Try replacing the redundant bits one at a time. For instance:

Ad.unverified_ads.size == 0 is equivalent to Ad.unverified_ads.blank?

And you're repeating "if...else" a lot with just one line in each, so turn it into a ternary:

if Ad.unverified_ads.size == 0 redirect_to verifications_path else redirect_to fast_verify_info_verification_path(Ad.unverified_ads.last) end

becomes

redirect_to (Ad.unverified_ads.blank? ? verifications_path : fast_verify_info_verification_path(Ad.unverified_ads.last))

Darek_Finster · January 20, 2012, 9:21pm

Thanks a lot, this my amendments. https://gist.github.com/1643261

11155 · January 20, 2012, 10:21pm

Thanks a lot.

I maked some improvments =>