4
\$\begingroup\$

I have this recursive function that searches an object tree structure:

dataSearcher = (dataElement, identifierToFind) -> 
    if dataElement.identifier == identifierToFind
        return dataElement
    else
        for childDataElement in dataElement.children
            found = dataSearcher childDataElement, identifierToFind
            if found then return found

Which I then call thus:

foundDataElement = dataSearcher @options.nodeData, identifier

It works just fine so I am happy about that, but I am pretty new to CoffeeScript and would like feedback on the way I structured it. The loop I used seems a bit old school, so could I have used a comprehension here instead? Any other feedback would be great as I am still getting my head into the CoffeeScript idiom.

Please let me know if I should edit with more context code.

\$\endgroup\$
  • 2
    \$\begingroup\$ You could make the for a comprehension, but it wouldn't cut the visit short when a match was found, so while this is less "functional" it's just as good. The real test of code is this: which do you expect to be able to read six months from now? I'd remove the 'else', but that's my Haskell training talking: you've got a guard condition at the top of your function, not an alternative, but that's how I read things. \$\endgroup\$ – Elf Sternberg Jan 6 '12 at 18:34
  • 1
    \$\begingroup\$ side note: the functional abstraction for what you are doing is mapDetect (see gist.github.com/1222480 for a underscore implementation). In a lazy language it would be (head . filter predicate). \$\endgroup\$ – tokland Jan 7 '12 at 14:42
6
\$\begingroup\$

I'd prefer writing it like this:

dataSearcher = (element, identifier) -> 
  return element if element.identifier is identifier
  for child in element.children
    found = dataSearcher child, identifier
    return found if found

Changes:

  • guard style instead of else (one less level of indentation)
  • postfix if (probably a question of style)
  • is instead of ==
  • shorter variable names
\$\endgroup\$
  • \$\begingroup\$ That is the sort of thing I am looking for...I am pretty clear I am still writing my coffeescript in a js idiom. Good coaching. \$\endgroup\$ – Matthew Nichols Jan 6 '12 at 23:29

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.