Any help with refactory this ugly code?

Rails
1

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?

2

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))

3

Thanks a lot.

I maked some improvments =>

#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

4

Thanks a lot, this my amendments.