Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I wrote this simple function in CoffeeScript to see if elements are found in an array.

isFoundIn = (searchTerm, arrayToSearch) ->
  returnVal = false
  for a in arrayToSearch
    returnVal = true if searchTerm is a
  returnVal

usage: isFoundIn('james', ['robert', 'michael', 'james']) would return true

I know in ruby it is very concise and readable to do this:

myArr = ['robert', 'michael', 'james']
myArr.include? 'james'

Are there some strategies to refactor this to make it more readable or more like CoffeeScript?

UPDATE:

Based on Flambino's suggestion I updated my approach:

Array::include = ((o) ->
  @indexOf(o) isnt -1
)

myArr = ['robert', 'michael', 'james']
myArr.include? 'james'
share|improve this question
    
It seems you've replicated the native JS array indexOf function. While there's no exact match for Ruby's include? in JavaScript, a simple array.indexOf(term) isnt -1 will give you the same result. –  Flambino May 13 '13 at 5:04
1  
Heh, just posted an answer with much the same stuff seconds after your update :) –  Flambino May 13 '13 at 6:28
1  
That's a reasonable question, but can infer from it that you're not using underscore.js? (or any other functional library for JS/CS). underscorejs.org/#contains. _(myArr).contains("james") #=> true. I imagine having to write JS without all those extensions and I get the chills... –  tokland May 13 '13 at 8:05

3 Answers 3

If you want to make it "more like CoffeeScript", use array comprehensions:

isFoundIn = (searchTerm, arrayToSearch) ->
  return true for item in arrayToSearch when item is searchTerm
  false
share|improve this answer

Expanding on my comment above:

Your isFoundIn function can be rewritten as simply

isFoundIn = (term, array) -> array.indexOf(term) isnt -1

indexOf uses the strict === comparison, which is what CoffeeScript also uses for is - i.e. it's equivalent to your code.

One could extend the native JS Array prototype with a include function to more closely mimic Ruby. In CoffeeScript, it'd be written as

Array::include = (term) -> @indexOf(term) isnt -1

Then you can do [1, 2, 3].include(3) and such. But extending native prototypes is generally frowned upon, and I can't recommend it. Tempting though.

As for your original function, you could just do an explicit return right away if the searchTerm is found - no need to loop through the rest of the array

    isFoundIn = (searchTerm, arrayToSearch) ->
      for a in arrayToSearch
        return true if searchTerm is a
      false

The point is moot, though, as indexOf does it all for you


Addendum: I do like the ? hack you used to fully match Ruby's method name, but it is still just that: A hack. You're effectively adding undefined as a possible return value for what should be a straight boolean. It'll also absorb errors (as that's its purpose) where e.g. the object responds to indexOf just fine, but - for whatever reason - hasn't been extended with the include function. So it's easy to get false negatives (unless you explicitly start checking for undefined and branch if that's the case and... well, the code fogs up quick)

So again, definite points for creativity - wish I'd thought of it - but I personally wouldn't go near it in production code

share|improve this answer
    
Why is extending prototypes generally frowned upon in JavaScript? Is it because you can create compatibility issues hypothetically with future releases of ECMASript and or other libraries? –  jamesstoneco May 13 '13 at 6:42
1  
@JamesJamesJames: Check SO, there are lots and lots of questions regarding this known issue: stackoverflow.com/questions/8859828/…. More orthodox is the "objects-wrapper" that many libraries take (jQuery being the most known example). –  tokland May 13 '13 at 8:09

Any reason to not use item in ary syntax? it compiles down to a slightly convoluted usage of .indexOf >=0 but it is the coffeescript way.

share|improve this answer
    
+1 for the coffeescript way. Don't know why I didn't think of that usage of in –  Flambino May 15 '13 at 14:29

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.