CSV lookup problem solved with CoffeeScript

Question

Source code with some test:

assert = require('assert')


compile_csv_search = (text, lookupKey)->
    [header, body...]= text.split('\n').filter(Boolean).map((el)->
        el.split(',')
    )
    col = header.indexOf(lookupKey)
    assert(col isnt -1)
    lookupIndex = body.reduce(
        (acc, row, i)->
            cell = row[col]
            acc[cell] = i
            return acc
        {}
    )
    return (lookupName)->
        rowNum = lookupIndex[lookupName]
        return body[rowNum].reduce(
            (acc, el, i)->
                acc[header[i]] = el
                return acc
            {}
        ) if rowNum?




# test one
(()->
    csvText = "ip,name,desc\n"+
        "10.49.1.4,server1,Main Server\n"+
        "10.52.5.1,server2,Backup Server\n"
    csv_by_name = compile_csv_search(
        csvText
        "name"
    )

    assert.deepEqual(
        csv_by_name("server2")
        {ip: "10.52.5.1", name: "server2", desc: "Backup Server"}
    )
    assert not csv_by_name("server9")?
)()

I think it is pretty descriptive.

Answer 1

Your lookupIndex may not work as intended unless the value you're using for the lookup is unique across all rows. You'll always find the last row in the CSV data, since the reduce function that creates lookupIndex will overwrite any existing lookup key/value with a new row number. That may not be a problem, but it is a limitation in your function.

You're doing a few things that you don't strictly need in CoffeeScript:

• Explicit returns
• Parentheses around function arguments
• A "JS-style" IIFE, when CoffeeScript has the do keyword
• You can define multi-line strings in CoffeeScript; just use 3 quotes (like Python)
• CoffeeScript also has a for..of object iterator which would probably be more readable than your reduce calls (although semantically different)

Admittedly, the first 2 post are as much a question of context and preference. Sometimes you do need parentheses (e.g. when chaining function calls), and sometimes an explicit return is more readable. Still, though, your syntax seems a little funky in some places (for instance omitting commas in the call that to compile_csv_list, yet including the parentheses?)

In more general terms:

• You're mixing snake_case and camelCase. Just stick to camelCase as that's the JS (and thus CoffeeScript) convention. For instance you'll see that assert.deepEqual is camel-cased. Speaking of convention; 2 spaces of indentation is the most common rule.

• You're requiring assert, presumably for use in your tests - but you also use it in the function itself. That seems like a bad idea. Especially since you can just throw your own, possibly more detailed, exception whenever you want.

• You could consider checking the header line for the lookupKey before parsing the rest of the lines.

• I hope your CSV data is fairly easy to deal with; otherwise I'd find a full featured CSV parsing library, if I were you. Right now, you're just splitting on newlines and commas - but CSV may contain strings with commas in them, and which aren't "cell" delimiters. And a string may contain quotes - in which case the outer string has two quotes around it or something, if I recall correctly... CSV is a bizarre format with no simple rules for escaping things. The only real way to parse it, if you want to be thorough, is to go byte by byte; a regex just won't cut it.

Anyway, here's a version with some style changes. The only functional difference is that it doesn't use assert in the function itself.

compileCSVSearch = (csv, lookupKey)->
  [header, rows...] = csv.split("\n").filter(Boolean).map (line) -> line.split ","

  columnIndex = header.indexOf lookupKey
  throw "Can't find a '#{lookupKey}' column" if column is -1 # consider throwin ag proper Error obj

  lookupTable = {}
  for row, i in rows
    value = rows[columnIndex]
    lookupTable[value] = i

  (lookupValue) ->
    index = lookupTable[lookupValue]
    return unless index?
    copy = {}
    copy[key] = value for own key, value of rows[index]
    copy


# Tests

do ->
  assert = require "assert"

  csvText = """ip,name,desc
               10.49.1.4,server1,Main Server
               10.52.5.1,server2,Backup Server\n"""

  csvByName = compileCSVSearch csvText, "name"

  assert.deepEqual csvByName("server2"),
    ip: "10.52.5.1",
    name: "server2",
    desc: "Backup Server"

  assert.equal csvByName("server9"), undefined