0
\$\begingroup\$

I was wondering if there was a better way to write this:

#Define Jquery Datatable
if $('#dtable').length
  table = $('#dtable').DataTable()

#If job is created, make new row in Jquery datatables
if data.status == "Created" then (
  count = 0
  new_data = [
    data.job_id
    data.start_ip
    data.end_ip
    data.ilo_username
    data.ilo_password
    data.status
    create_date
    '<a class="btn btn-small btn-info" href="/ilo_scan_jobs/' + data.job_id + '">Details</a>'
  ]
  new_row = table.row.add(new_data).draw().nodes().to$().find('td').each ->
    $(this).attr 'id', 'td_' + count++ + '_' + data.job_id
  new_row
  id = $('#row' + data.job_id).val()
  table.row(new_row).node().id = 'row' + data.job_id
)

#If job status is running
else if data.status != "Created" and data.status != "Job Deleted" and data.status != "Scan Complete" then (
  #Update Jquery datatable if it exists
  if $('#dtable').length
    table.cell('#td_5_' + data.job_id).data wait_status

  #Update job detail status if it exists
  if $(detail_status).length
    $(detail_status).html wait_status

  #Detail Status  
  if $(respond_message).length then (
    if data.count == null or data.server_count != 0
      $(respond_message).html wait_message
    else if data.server_count == 0
      $(respond_message).html no_respond
  )
) 
#If job status is complete
else if data.status == "Scan Complete" then (
  #Update Jquery Datatable if it exists
  if $('#dtable').length
    table.cell('#td_5_' + data.job_id).data(data.status)

  #Update job detail status if it exists
  if $(detail_status).length
    $(detail_status).html data.status

  #Detail Status  
  if $(respond_message).length then (
    if data.server_count == 0
      $(respond_message).html no_respond
    else if data.server_count != 0
      $(respond_message).html respond_finish
  )

#Remove disabled class on buttons if exists
  if $('#edit_disable_' + data.job_id).length and data.status == 'Scan Complete'
    $('#edit_disable_' + data.job_id).removeClass 'disabled'
  if $('#delete_disable_' + data.job_id).length and data.status == 'Scan Complete'
    $('#delete_disable_' + data.job_id).removeClass 'disabled'
)
#If job is deleted, remove table row from Jquery datatable
else if data.status == "Job Deleted"
  tr = "#row" + data.job_id
  table.row(tr).remove().draw()

Even though it is working, the code seems a little sloppy to me with all the if/else statements. Is there any way to simplify it? I'm new to coffeescript.

\$\endgroup\$
  • \$\begingroup\$ There are few things that can be shortened, but first things first - what is the detail_status? I don't see its declaration anywhere. \$\endgroup\$ – fracz Feb 6 '17 at 15:30
2
\$\begingroup\$

First of all, you may benefit from the fact that in Coffee (and in JS, too), the following code

if condition
  variable = value

can be expressed simpler as

variable = condition && value

Therefore, the declaration of the table may be written as

table = $('#dtable').length && $('#dtable').DataTable()

Later in the code, you should not check again for the $('#dtable').length but you should check if the table has a value, possibly with the existential operator. For example, replace the

if $('#dtable').length
    table.cell('#td_5_' + data.job_id).data wait_status

with

table?.cell('#td_5_' + data.job_id).data wait_status

I don't know what the detail_status is but I suppose it is a string selector. Therefore, you don't need to check if it is available in the DOM. JQuery will do nothing if there is no such element, so you can safely call $(detail_status).html wait_status without checking for its length. The same goes for e.g. removeClass method.

You can also use Coffee's string interpolation and replace the "#row" + data.job_id with "#row#{data.job_id}" (and similar).

Last but not least, try replacing multiple conditions like if data.status != "Created" and data.status != "Job Deleted" and data.status != "Scan Complete" with one statement with the in operator, i.e. if data.status not in ['Created', 'Job Deleted', 'Scan Complete'].

\$\endgroup\$
  • \$\begingroup\$ Thanks for the info fracz. After defining table into a condition and value as you suggested, I tried table?.cell('#td_5_' + data.job_id).data wait_status, but I am getting the error that table.cell is not a function. Still playing around with it... \$\endgroup\$ – Wes Feb 6 '17 at 21:23
  • \$\begingroup\$ Also tried table.cell('#td_5_' + data.job_id).data wait_status if table? but still get the same result. The problem appears to be that it is running table.cell without first checking if it exists. \$\endgroup\$ – Wes Feb 6 '17 at 21:30
  • \$\begingroup\$ My fault. If there is no #dtable, after the table = $('#dtable').length && $('#dtable').DataTable() the table gets value 0, which passes the existential operator (the value exists). Replace it with table = $('#dtable').DataTable() if $('#dtable').length and the rest of the code should work as I have written. \$\endgroup\$ – fracz Feb 6 '17 at 21:53
  • \$\begingroup\$ Thanks - that did the trick. I'll continue to try and clean it up. This helped a lot. \$\endgroup\$ – Wes Feb 6 '17 at 22:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.