CODEWITHSUNDEEP
ruby-on-railscontrollerrefactoring
NdaJunior
NdaJunior

Reputation: 374

Any idea how I can refactor this any further

I'm new to rails and i've really tried my best to keep refactoring this method but this is the smallest I can get it to. Any ideas as to what i can extract or refactor? etc

def create
 if verify_recaptcha
  super do |resource|
    if resource.save
      pin = resource.create_pin(otp: new_otp)
      begin
        SendMessage.new({pin: pin.otp, contact_number: resource.contact_number, first_name: resource.first_name}).send_otp
      rescue Exceptions::ServiceDownError
        sign_in(resource)
        return redirect_to users_otp_verification_path(user: @user.uuid, service_status: true)
      end
      sign_in(resource)
      return redirect_to users_otp_verification_path(user: @user.uuid)
    else
      return render :new, resource: resource
    end
  end
  else
     flash.now[:alert] = 'There was an error with the recaptcha verification. Please verify that you are human.'
     flash.delete :recaptcha_error
     return render :new, resource: build_resource(hash = nil)
  end
end

Upvotes: 0

Views: 41

Answers (1)

alebian
alebian

Reputation: 782

To start you can do:

def create
  if verify_recaptcha
    super do |resource|
      return render :new, resource: resource unless resource.save
      begin
        SendMessage.new({pin: resource.create_pin(otp: new_otp).otp, contact_number: resource.contact_number, first_name: resource.first_name}).send_otp
      rescue Exceptions::ServiceDownError
        sign_in(resource)
        return redirect_to users_otp_verification_path(user: @user.uuid, service_status: true)
      end
      sign_in(resource)
      return redirect_to users_otp_verification_path(user: @user.uuid)
    end
  else
    flash.now[:alert] = 'There was an error with the recaptcha verification. Please verify that you are human.'
    flash.delete :recaptcha_error
    return render :new, resource: build_resource(hash = nil)
  end
end

I don't know what 'service_status: true' means inside a ServiceDownError... but I guess you can do:

def create
  if verify_recaptcha
    super do |resource|
      return render :new, resource: resource unless resource.save
      begin
        SendMessage.new({pin: resource.create_pin(otp: new_otp).otp, contact_number: resource.contact_number, first_name: resource.first_name}).send_otp
      ensure
        sign_in(resource)
        return redirect_to users_otp_verification_path(user: @user.uuid, service_status: true)
      end
    end
  else
    flash.now[:alert] = 'There was an error with the recaptcha verification. Please verify that you are human.'
    flash.delete :recaptcha_error
    return render :new, resource: build_resource(hash = nil)
  end
end

Or maybe:

def create
  if verify_recaptcha
    super do |resource|
      return render :new, resource: resource unless resource.save
      status = nil
      begin
        SendMessage.new({pin: resource.create_pin(otp: new_otp).otp, contact_number: resource.contact_number, first_name: resource.first_name}).send_otp
      rescue Exceptions::ServiceDownError
        status = true
      ensure
        sign_in(resource)
        return redirect_to users_otp_verification_path(user: @user.uuid, status: status)
      end
    end
  else
    flash.now[:alert] = 'There was an error with the recaptcha verification. Please verify that you are human.'
    flash.delete :recaptcha_error
    return render :new, resource: build_resource(hash = nil)
  end
end

To go further you could extract this in a separate method:

{pin: resource.create_pin(otp: new_otp).otp, contact_number: resource.contact_number, first_name: resource.first_name}

like:

private

def message_params(resource)
  {
    pin: resource.create_pin(otp: new_otp).otp,
    contact_number: resource.contact_number,
    first_name: resource.first_name
  }
end

Remember that refactoring is not just about making the code small, but also making it cleaner and easier to read, so extracting things to smaller methods can be a good idea

Upvotes: 1

Related Questions