patternModerate
Optimize CoffeeScript comparison function
Viewed 0 times
coffeescriptfunctionoptimizecomparison
Problem
I've wrote this CoffeScript function to compare values in a table, but it turned out to be a HUGE function.
Since the code is really really long, you can find it here as well.
Code explanation:
The code searches for all the values in a dynamically created datatable and appends the comparison values in another column, Moreover, I had to clone the same statements (because the number of columns is different based on window size) to get it to work on mobile as well.
Is there any way I can reduce the size of this function?
```
comparekeys = ->
if $("#keys tbody tr").length > 1
$.expr[":"].exactly = (e, i, m) ->
$(e).text() is m[3]
$("#keys tbody tr td:nth-child(2):exactly(0)").html ""
$("#keys tbody tr").each(->
width = $(window).width()
if width > 1025
a = Number($(this).find("td")[1].innerHTML)
b = Number($(this).find("td")[3].innerHTML)
c = Number($(this).find("td")[4].innerHTML)
d = Number($(this).find("td")[5].innerHTML)
changes = $(this).find("td:eq(2)")
row1 = $(this).find("td:eq(1)")
row2 = $(this).find("td:eq(3)")
row3 = $(this).find("td:eq(4)")
row4 = $(this).find("td:eq(5)")
css = 'color':'white', 'cursor':'default'
notranking = " Not Ranking "
notranking_today = " Not Ranking "
sortboxarrowup = "sortboxup fa fa-arrow-up"
sortboxarrowdown = "sortboxdown fa fa-arrow-down"
sortboxbarseq = "sortboxeq fa fa-bars"
$("[rel=tooltip]").tooltip placement: "top"
$("#keys > tbody > tr > td.sorting_1 > a").attr
rel: "tooltip"
title: "View keyword data"
if a is 0
if b > 0
$(" Dropped ").addClass("sortboxdrop").css(css).appendTo(row1)
else
$(" Not Ranking ").addClass("sortboxeq").css(css).appendTo(row1)
# else if a > 0 && b is 0 && c is 0 && d is 0
Since the code is really really long, you can find it here as well.
Code explanation:
The code searches for all the values in a dynamically created datatable and appends the comparison values in another column, Moreover, I had to clone the same statements (because the number of columns is different based on window size) to get it to work on mobile as well.
Is there any way I can reduce the size of this function?
```
comparekeys = ->
if $("#keys tbody tr").length > 1
$.expr[":"].exactly = (e, i, m) ->
$(e).text() is m[3]
$("#keys tbody tr td:nth-child(2):exactly(0)").html ""
$("#keys tbody tr").each(->
width = $(window).width()
if width > 1025
a = Number($(this).find("td")[1].innerHTML)
b = Number($(this).find("td")[3].innerHTML)
c = Number($(this).find("td")[4].innerHTML)
d = Number($(this).find("td")[5].innerHTML)
changes = $(this).find("td:eq(2)")
row1 = $(this).find("td:eq(1)")
row2 = $(this).find("td:eq(3)")
row3 = $(this).find("td:eq(4)")
row4 = $(this).find("td:eq(5)")
css = 'color':'white', 'cursor':'default'
notranking = " Not Ranking "
notranking_today = " Not Ranking "
sortboxarrowup = "sortboxup fa fa-arrow-up"
sortboxarrowdown = "sortboxdown fa fa-arrow-down"
sortboxbarseq = "sortboxeq fa fa-bars"
$("[rel=tooltip]").tooltip placement: "top"
$("#keys > tbody > tr > td.sorting_1 > a").attr
rel: "tooltip"
title: "View keyword data"
if a is 0
if b > 0
$(" Dropped ").addClass("sortboxdrop").css(css).appendTo(row1)
else
$(" Not Ranking ").addClass("sortboxeq").css(css).appendTo(row1)
# else if a > 0 && b is 0 && c is 0 && d is 0
Solution
You don't seem to have a representation of your table as data, just in the DOM.
That's making this a really cumbersome job. This will be more legible if you
follow the Rule of Representation.
Represent your table as an array of objects. Make judgements about those
objects based on their properties, and then render them as DOM elements.
It should go something like this:
Get your rows as data.
Add properties you need for rendering to the rows.
decreased: 'sortboxdown fa fa-arrow-down'
same: 'sortboxeq fa fa-bars'
# ... etc
row.className = cssClasses[row.change] for row in rows
$("").attr
'rel': 'tooltip'
'title': row.title
'class': row.className
$('body').append (renderRow row for row in rows)
# event handling code here
td {
color: white;
cursor: default;
}
`
That's making this a really cumbersome job. This will be more legible if you
follow the Rule of Representation.
Represent your table as an array of objects. Make judgements about those
objects based on their properties, and then render them as DOM elements.
It should go something like this:
Get your rows as data.
# these can come from an api, database, or wherever they're coming from now
rows = [
{today: 1, yesterday: 0, started: 0}
{today: 2, yesterday: 4, started: 1}
# ... etc
]
Add properties you need for rendering to the rows.
computeChange = (row) ->
[a, b] = [row.today, row.yesterday]
if a > b and b > 0
'decreased'
else if a > b and b is 0
'new'
else if a 0
'dropped'
else if a
You can add css classes in a similar fashion.
cssClasses =decreased: 'sortboxdown fa fa-arrow-down'
same: 'sortboxeq fa fa-bars'
# ... etc
row.className = cssClasses[row.change] for row in rows
Once every row have enough information to be rendered, pass the to something
that draws rows.
renderRow = (row) ->$("").attr
'rel': 'tooltip'
'title': row.title
'class': row.className
$('body').append (renderRow row for row in rows)
Delegate events to an element that contains everything you drew
$('body').on 'change', 'select[name=keycompare]', (event) -># event handling code here
Styling that you want on every element should be added to the document, not each element.
-->td {
color: white;
cursor: default;
}
`
Context
StackExchange Code Review Q#51293, answer score: 13
Revisions (0)
No revisions yet.