HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Periodic Table generator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
periodictablegenerator

Problem

I've been playing around with Jekyll and CoffeeScript the last few days and made a Periodic Table to learn those two.

Now, to create the Periodic Table, I take an array of lis as input which then replaces itself with a table using a custom format like so:

periodicTable = [
    [1, "s:16", 2],
    [3, 4, "s:10", 5, "-", 10],
    [11, 12, "s:10", 13, "-", 18],
    [19, "-", 36],
    [37, "-", 54],
    [55, 56, "*", 72, "-", 86],
    [87, 88, "**", 104, "-", 118],
    ["s:18"],
    ["s:2", "*", 57, "-", 71],
    ["s:2", "**", 89, "-", 103]
]


  • s:n means that the next td should be empty with a colspan of n



  • "-" is a range indicator, meaning the next and previous items are the start and end of the range respectively



To create the actual table, the following CoffeeScript file is used:

```
window.PeriodicTable = {}

periodicTable = [
[1, "s:16", 2],
[3, 4, "s:10", 5, "-", 10],
[11, 12, "s:10", 13, "-", 18],
[19, "-", 36],
[37, "-", 54],
[55, 56, "*", 72, "-", 86],
[87, 88, "**", 104, "-", 118],
["s:18"],
["s:2", "*", 57, "-", 71],
["s:2", "**", 89, "-", 103]
]

spacerIdentifier = "s"
rangeSymbol = "-"

# Get a list item searching by atomic number
PeriodicTable.getListItemByAtomicNumber = (list, atomicNumber) ->
i = 0
while i
table = document.createElement "table"
table.className = "periodic-table"

ul = list[0].parentNode

for i of periodicTable
row = PeriodicTable.createRow list, periodicTable[i]
table.appendChild row

ul.parentNode.insertBefore table, ul

return true

# Create a table row
PeriodicTable.createRow = (list, items) ->
row = document.createElement "tr"

i = 0
skipNext = false

while i
col = document.createElement "td"

if !isNaN atomicNumber
listContent = PeriodicTable.getContentFromListItem(list, atomicNumber)

if typeof listContent == "string"
content = listContent

else
c

Solution

CoffeeScript or otherwise, it does seem like you're working a bit too hard here.

Since the information you use is split between the list in the HTML and the array in the script, there's some coupling between the two and some busywork in weaving the two together. Optimally, you'd have all the information in one place to begin with, but I figure the idea is that the list already exists, and you're trying to lay it out as a table.

But in order to get closer to the single source of data, we can parse the list and the layout array first, and then go about building the table from that.

In the case of the array, we just want to expand the ranges (leaving spaces as-is), and for the list, we want to extract the stuff we need from content and data-* attributes and, say, put it in an object using numbers as keys.

Basically, instead of trying to do everything while building the table, we prepare the data first.

In terms of style, you're following good JavaScript style by creating a PeriodicTable namespace, but that isn't really necessary (at least not in the same way) in CoffeeScript. Everything is compiled inside an IIFE, so you can treat everything as local as simply expose what you want to expose. E.g.:

foo = -> ...
bar = -> ...
baz = -> ...

# expose some functions (if necessary)
window.someNamespace = {foo, bar}


The baz function will remain "private", while foo and bar will become available globally as someNamespace.foo and someNamespace.bar respectively. Meanwhile, within your script, you don't have to worry about prefixing everything with a namespace.

In this case, though, I'm not sure you actually need to expose anything - it seems pretty self-contained.

You've also got a bunch of explicit return keywords which aren't necessary.

The table you're building also contains invalid HTML. You're wrapping p elements in an a element, which isn't quite kosher, since a is an inline element, and p is a block-level element. You've also got bunch of span elements, which I'm not sure are necessary. They just seem to add an extra level to the nesting.

Lastly, I'd suggest using jQuery since you're dealing with the DOM, and perhaps underscore.js or lo-dash to help with the parsing. I'll use jQuery below just to simplify element construction.

The array can be expanded, row by row, like so (hey that rhymes)

expandLayout = (layout) ->
  # internal function to expand a row
  expandRow = (row) ->
    expanded = []
    for item, index in row
      if item is '-'
        start = expanded.pop() # previous number
        end   = row[index+1]   # next number
        range = [start...end]  # range excluding end
        expanded.push range...
      else
        expanded.push item
    expanded

  # expand the layout (returns a new array)
  expandRow row for row in layout


(You could also use reduce, but the syntax is kinda messy when you want to use an initial value for the accumulator.) I've left the spacer-items alone, though.

The list can be parsed pretty much how you're doing already, just collecting the data in an object.

In all, I get something like this

```
TABLE_LAYOUT = [
[1, "s:16", 2],
[3, 4, "s:10", 5, "-", 10],
[11, 12, "s:10", 13, "-", 18],
[19, "-", 36],
[37, "-", 54],
[55, 56, "*", 72, "-", 86],
[87, 88, "**", 104, "-", 118],
["s:18"],
["s:2", "*", 57, "-", 71],
["s:2", "**", 89, "-", 103]
]

# expand ranges in the layout array
expandLayout = (layout) ->
expandRow = (row) ->
expanded = []
for item, index in row
if item is '-'
start = expanded.pop()
end = row[index+1]
range = [start...end]
expanded.push range...
else
expanded.push item
expanded

expandRow row for row in layout

# extract data from a list of elements
parseList = (list) ->
elements = {}
list.children('li').each ->
item = $ this
number = item.data 'number' # seems a better name than "atomicNumber"
return unless number? # skip if the item isn't actually an element
elements[number] =
symbol: item.data 'symbol'
type: item.data 'type' # again, simpler name
name: item.text()
url: item.first('a').attr 'href'
elements

# build the periodic table
buildTable = (elements, layout) ->
typeToClassName = (type) ->
type.toLowerCase().replace /\s+/g, '-'

buildCell = (value) ->
cell = $ ''

if match = String(value).match /^s:(\d+)$/ # spacer cell
cell.attr 'colspan', match[1]

else if element = elements[value] # element cell
link = $('').attr 'href', element.url
link.append $('').text value
link.append $('').text element.name
link.append $('').text element.symbol
cell.addClass(typeToClassName element.type).append link

else # text cell
cell.text value

cell

buildRow = (row) ->
tr = $ ''
tr.append buildCell(item) for item in row
tr

table = $ ''
table.append buildRow(row) for row in layout

table

Code Snippets

foo = -> ...
bar = -> ...
baz = -> ...

# expose some functions (if necessary)
window.someNamespace = {foo, bar}
expandLayout = (layout) ->
  # internal function to expand a row
  expandRow = (row) ->
    expanded = []
    for item, index in row
      if item is '-'
        start = expanded.pop() # previous number
        end   = row[index+1]   # next number
        range = [start...end]  # range excluding end
        expanded.push range...
      else
        expanded.push item
    expanded

  # expand the layout (returns a new array)
  expandRow row for row in layout
TABLE_LAYOUT = [
  [1, "s:16", 2],
  [3, 4, "s:10", 5, "-", 10],
  [11, 12, "s:10", 13, "-", 18],
  [19, "-", 36],
  [37, "-", 54],
  [55, 56, "*", 72, "-", 86],
  [87, 88, "**", 104, "-", 118],
  ["s:18"],
  ["s:2", "*", 57, "-", 71],
  ["s:2", "**", 89, "-", 103]
]

# expand ranges in the layout array
expandLayout = (layout) ->
  expandRow = (row) ->
    expanded = []
    for item, index in row
      if item is '-'
        start = expanded.pop()
        end   = row[index+1]
        range = [start...end]
        expanded.push range...
      else
        expanded.push item
    expanded

  expandRow row for row in layout

# extract data from a list of elements
parseList = (list) ->
  elements = {}
  list.children('li').each ->
    item = $ this
    number = item.data 'number' # seems a better name than "atomicNumber"
    return unless number? # skip if the item isn't actually an element
    elements[number] =
      symbol: item.data 'symbol'
      type: item.data 'type' # again, simpler name
      name: item.text()
      url: item.first('a').attr 'href'
  elements

# build the periodic table
buildTable = (elements, layout) ->
  typeToClassName = (type) ->
    type.toLowerCase().replace /\s+/g, '-'

  buildCell = (value) ->
    cell = $ '<td></td>'

    if match = String(value).match /^s:(\d+)$/ # spacer cell
      cell.attr 'colspan', match[1]

    else if element = elements[value] # element cell
      link = $('<a></a>').attr 'href', element.url
      link.append $('<span></span>').text value
      link.append $('<span></span>').text element.name
      link.append $('<span></span>').text element.symbol
      cell.addClass(typeToClassName element.type).append link

    else # text cell
      cell.text value

    cell

  buildRow = (row) ->
    tr = $ '<tr></tr>'
    tr.append buildCell(item) for item in row
    tr

  table = $ '<table></table>'
  table.append buildRow(row) for row in layout

  table


# on load, replace the list
$ ->
  list = $ '#elements'
  layout = expandLayout TABLE_LAYOUT
  elements = parseList list
  list.replaceWith buildTable(elements, layout)

Context

StackExchange Code Review Q#74876, answer score: 2

Revisions (0)

No revisions yet.