CODEWITHSUNDEEP
ruby-on-railscoffeescript
tmartin314
tmartin314

Reputation: 4171

Refactoring coffee script method

I have put this method together to achieve some basic validation in a rails app.

I'm very new to rails/coffeescript and wondering if anyone has ideas on refactoring/simplifying it:

  validateBillingAddress: (event) ->
    add1 = $('#user_billing_address_1')
    city = $('#user_billing_city')
    zip = $('#user_billing_zip')

    add1.removeClass('error')
    city.removeClass('error')
    zip.removeClass('error')

    if !$('#user_billing_agreement').is(':checked')
      $('button[type=submit]').removeAttr('disabled')
      alert('You must agree to the subscription')
      return

    if !add1.val().length
      $('button[type=submit]').removeAttr('disabled')
      add1.addClass('error')
      return

    else if !city.val().length
      $('button[type=submit]').removeAttr('disabled')
      city.addClass('error')
      return

    else if !zip.val().length
      $('button[type=submit]').removeAttr('disabled')
      zip.addClass('error')
      return
    else 
      @processCard()

    event.preventDefault()

Upvotes: 0

Views: 113

Answers (2)

Drew
Drew

Reputation: 738

Here is my take on it.

Very small, single-concern functions. This might be overkill, but I've found that most applications will grow in complexity, and getting a clean structure early helps later.

Also it sets the stage for reusing this code later on other forms. Perhaps even making a coffeescript class.

#
# Handle submit event, validate, and submit
#
$('form').on "submit", ->
  if isValidBillingAddress() and isAgreementChecked()
    @processCard()

#
# Validation checks
#
isAgreementChecked = ->
  if $('#user_billing_agreement').is(':checked')
    true
  else
    alert('You must agree to the subscription')
    false

isValidBillingAddress = ->
  enableForm()
  for field in requiredFormFields()
    do (field) ->
      if isInvalidData(field)
        showErrorOnField(field)
      else
        field.removeClass('error')

#
# Support functions
#
requiredFormFields = ->
  address1 = $("#user_billing_address_1")
  city = $("#user_billing_city")
  zip = $("#user_billing_zip")
  [address1, city, zip]

isInvalidData = (field) ->
  field.val() is ""

showErrorOnField = (field) ->
  field.addClass('error')

enableForm = ->
  $('button[type=submit]').removeAttr('disabled')

Upvotes: 1

hedgesky
hedgesky

Reputation: 3311

I think you could try something like this (not tested)

validateBillingAddress: (event) ->
  event.preventDefault()
  fields = $('#user_billing_address_1, #user_billing_city, #user_billing_zip')

  fields.removeClass('error')

  unless $('#user_billing_agreement').is(':checked')
    $('button[type=submit]').removeAttr('disabled')
    alert('You must agree to the subscription')
    return

  fields.each ->
    if !$(this).val().length
      $('button[type=submit]').removeAttr('disabled')
      $(this).addClass('error')

  if fields.filter('.error').length > 0
    return
  else
    @processCard()

Upvotes: 3

Related Questions