3
\$\begingroup\$

I am working on a simple Rails app. For the Expense resource I use a DataTable. So, in my CoffeeScript for this resource I basically do several things:

  • initialize the datatabe;
  • since I use a custom search field (not the one, provided by DataTables), I add a search function in the table api;
  • initialize the Select2 plugin;
  • check to see if the request is made from a mobile device - if yes -> change the type of an input field to date, otherwise initialize Datepicker
  • submit a from when the date from the datepicker is selected

Here is my code

$.fn.dataTable.ext.type.order['currency-bg-pre'] = parseCurrency
changeTextInputToDate = (input) ->
 input.each(-> $(@).clone().attr('type', 'date').insertBefore(@)).remove()
dataTableFooterCallback = (row, data, start, end, display) ->
 api = @api()
 total = api.column(1).data().reduce sumCurrency, 0
 $(api.column(1).footer()).html("#{total.toFixed(2)} лв")
initializeDatepicker = (element) ->
 element.datepicker
 autoclose: true
 clearBtn: true
 disableTouchKeyboard: true
 format: 'yyyy-mm-dd'
 orientation: 'top auto'
 todayBtn: true
 todayHighlight: true
initializeDateTable = (element) ->
 element.DataTable
 columns: [
 type: 'date',
 searchable: false
 ,
 type: 'currency-bg'
 searchable: false
 ,
 type: 'string'
 ,
 type: null
 orderable: false
 searchable: false
 ]
 footerCallback: dataTableFooterCallback
 order: [[0, 'desc']]
 paging: false
 dom: 't'
initializeSelect2 = (element) ->
 element.select2
 ajax:
 url: '/tags'
 data:
 (query) ->
 query: query.term
 processResults:
 (data) ->
 results: data
 cache: true
 placeholder: 'Enter tag(s)'
 tags: true
 theme: 'bootstrap'
 tokenSeparators: [',']
isMobile = ->
 check = false
 ((a) ->
 if /(android|bb\d+|meego).+mobile|avantgo|bada\/|blackberry|blazer|compal|elaine|fennec|hiptop|iemobile|ip(hone|od)|iris|kindle|lge |maemo|midp|mmp|mobile.+firefox|netfront|opera m(ob|in)i|palm( os)?|phone|p(ixi|re)\/|plucker|pocket|psp|series(4|6)0|symbian|treo|up\.(browser|link)|vodafone|wap|windows ce|xda|xiino/i.test(a) or /1207|6310|6590|3gso|4thp|50[1-6]i|770s|802s|a wa|abac|ac(er|oo|s\-)|ai(ko|rn)|al(av|ca|co)|amoi|an(ex|ny|yw)|aptu|ar(ch|go)|as(te|us)|attw|au(di|\-m|r |s )|avan|be(ck|ll|nq)|bi(lb|rd)|bl(ac|az)|br(e|v)w|bumb|bw\-(n|u)|c55\/|capi|ccwa|cdm\-|cell|chtm|cldc|cmd\-|co(mp|nd)|craw|da(it|ll|ng)|dbte|dc\-s|devi|dica|dmob|do(c|p)o|ds(12|\-d)|el(49|ai)|em(l2|ul)|er(ic|k0)|esl8|ez([4-7]0|os|wa|ze)|fetc|fly(\-|_)|g1 u|g560|gene|gf\-5|g\-mo|go(\.w|od)|gr(ad|un)|haie|hcit|hd\-(m|p|t)|hei\-|hi(pt|ta)|hp( i|ip)|hs\-c|ht(c(\-| |_|a|g|p|s|t)|tp)|hu(aw|tc)|i\-(20|go|ma)|i230|iac( |\-|\/)|ibro|idea|ig01|ikom|im1k|inno|ipaq|iris|ja(t|v)a|jbro|jemu|jigs|kddi|keji|kgt( |\/)|klon|kpt |kwc\-|kyo(c|k)|le(no|xi)|lg( g|\/(k|l|u)|50|54|\-[a-w])|libw|lynx|m1\-w|m3ga|m50\/|ma(te|ui|xo)|mc(01|21|ca)|m\-cr|me(rc|ri)|mi(o8|oa|ts)|mmef|mo(01|02|bi|de|do|t(\-| |o|v)|zz)|mt(50|p1|v )|mwbp|mywa|n10[0-2]|n20[2-3]|n30(0|2)|n50(0|2|5)|n7(0(0|1)|10)|ne((c|m)\-|on|tf|wf|wg|wt)|nok(6|i)|nzph|o2im|op(ti|wv)|oran|owg1|p800|pan(a|d|t)|pdxg|pg(13|\-([1-8]|c))|phil|pire|pl(ay|uc)|pn\-2|po(ck|rt|se)|prox|psio|pt\-g|qa\-a|qc(07|12|21|32|60|\-[2-7]|i\-)|qtek|r380|r600|raks|rim9|ro(ve|zo)|s55\/|sa(ge|ma|mm|ms|ny|va)|sc(01|h\-|oo|p\-)|sdk\/|se(c(\-|0|1)|47|mc|nd|ri)|sgh\-|shar|sie(\-|m)|sk\-0|sl(45|id)|sm(al|ar|b3|it|t5)|so(ft|ny)|sp(01|h\-|v\-|v )|sy(01|mb)|t2(18|50)|t6(00|10|18)|ta(gt|lk)|tcl\-|tdg\-|tel(i|m)|tim\-|t\-mo|to(pl|sh)|ts(70|m\-|m3|m5)|tx\-9|up(\.b|g1|si)|utst|v400|v750|veri|vi(rg|te)|vk(40|5[0-3]|\-v)|vm40|voda|vulc|vx(52|53|60|61|70|80|81|83|85|98)|w3c(\-| )|webc|whit|wi(g |nc|nw)|wmlb|wonu|x700|yas\-|your|zeto|zte\-/i.test(a.substr(0, 4))
 check = true
 return
 ) navigator.userAgent or navigator.vendor or window.opera
 check
parseCurrency = (amount) ->
 switch typeof amount
 when 'string' then amount.match(/\d+(?:\.\d+)?/)[0] * 1
 when 'number' then amount
 else 0
sumCurrency = (a, b) -> parseCurrency(a) + parseCurrency(b)
$ ->
 api = initializeDateTable $('.datatable')
 $('#search').on 'keyup', -> api.search(@value).draw()
 initializeSelect2 $('#expense_tag_list')
 if isMobile()
 changeTextInputToDate $('form input:text')
 else
 initializeDatepicker $('.input-group.date')
 $('.datepicker').on 'change', -> $('form').submit()

So what I do is basically this:

  • figure out what must happen on the page;
  • write the function that does that;
  • call it on page ready;

Although I have provided a particular example, I generally write JavaScript/CoffeeScript this way. Would you provide some basic example of how you would reorganize this code. In particular I am interested in using CoffeeScript's classes. Also, at what point does one decide that the code has become too messy and one needs to start using Angular/React etc.?

200_success
145k22 gold badges190 silver badges478 bronze badges
asked Nov 6, 2015 at 11:55
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

I can't tell you when something like Angular/React will make more sense. It's a cost/benefit question, but they're your costs and your benefits.

I'd say you code is okay. In some places it seems overly terse, while in others it's spread out too much. And there are a few places where you perhaps abuse CoffeeScript syntax to the point where it becomes harder to read.

There's also a number of things that can just be simplified:

  • The isMobile function doesn't need to be a function with a nested IIFE and a closure. Just run it:

    isMobile = do ->
     userAgent = navigator.userAgent or navigator.vendor or window.opera or ""
     /.../.test(userAgent) or /.../.test(userAgent.substr(0,4))
    

    Now, isMobile is just a boolean value. The do keyword means it's evaluated immediately. Note I've also added a "" fallback for userAgent just in case. Since you might call substr on userAgent, it must be a string, or the script will just fail.

  • You changeTextInputToDate doesn't actually change the inputs, really. It replaces them. But changing them seems much easier:

    changeTextInputToDate = (inputs) ->
     inputs.each -> $(this).attr type: 'date'
    

    (I'm using this instead of @ just because I always think @ looks weird by itself. I like using it as a prefix for things, but if I'm referring to the this object itself, I prefer to use this. Just personal preference.)

  • In dataTableFooterCallback it'd be nicer to fetch the column once, since that's what you need for the expressions. I'd also make the footer callback a nested function in initializeDataTable, too. Especially as that function references @api, which doesn't make sense except in the context of a data table callback. So there's no reason for the function to float around in a wider scope:

    initializeDateTable = (element) ->
     footerCallback = ->
     column = @api().column(1)
     total = column.data().map(parseCurrency).reduce (sum, value) -> sum + value
     $(column.footer()).html "#{total.toFixed(2)} лв"
     element.DataTable
     columns: [
     { type: 'date', searchable: false }
     { type: 'currency-bg', searchable: false }
     { type: 'string' }
     { type: null, orderable: false, searchable: false }
     ]
     footerCallback: footerCallback
     order: [[0, 'desc']]
     paging: false
     dom: 't'
    

    You'll note a few more changes:

    • I'm using explicit { and } around objects in the columns array, rather than relying solely on whitespace - makes it easier to read, I think.
    • I'm using map + reduce, rather than doing all the work in the reduce callback. This makes for a simpler expression, and eliminates sumCurrency completely.
  • parseCurrency I'd write as

    parseCurrency = (value) ->
     switch typeof value
     when 'number' then value
     when 'string' then (value.match(/\d+(\.\d+)?/)?[0] or 0) * 1
     else 0
    

    The ?[0] or 0 guards against match returning null. Without the null-coalescing ? your script just fails as null[0] doesn't work.

Finally, I'm not sure I understand these last lines:

if isMobile()
 changeTextInputToDate $('form input:text')
else
 initializeDatepicker $('.input-group.date')
$('.datepicker').on 'change', -> $('form').submit()

So, if it's a mobile client you change all text inputs in any form. If it's not, you initialize a datepicker with a completely different - and much more specific - selector. It just reads as "either do this, or do something completely different". Why aren't the selectors more similar? I'd imagine it's the same inputs you're dealing with.

And finally, the change event listener. That's yet another selector!

So that's confusing.

answered Nov 17, 2015 at 20:36
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.