4 ms·
Big, big fan of CoffeeScript and glad to see Dropbox hopping aboard. That being said, some of their examples are lackluster. @originalStyle = {} for k in
by sync 14y ago
Big, big fan of CoffeeScript and glad to see Dropbox hopping aboard.
That being said, some of their examples are lackluster.
@originalStyle = {}
for k in ['top', 'left', 'width', 'height']
@originalStyle[k] = @element.style[k]
Should really be something like:
@originalStyle = ['top', 'left', 'width', 'height'].reduce (hash, position) ->
hash[position] = @element.style[position]
hash
, {}
... though that shows off some CoffeeScript warts.
Also,
Sharing =
init: (sf_info) ->
for list in [sf_info.current, sf_info.past]
for info in list
@_decode_sort_key info
Why aren't they using CoffeeScript classes?
class Sharing
constructor: (sfInfo) ->
...
- gfodor 14y agoOn the former, I think you'll find most people find their implementation more readable/idiomatic, and the second case I'd guess is because they were doing a port and so probably didn't want to make too many semantic leaps.
- alexangelini 14y agoYour first example is the exact reason I dislike CoffeeScript. Sure it's clever, but at a glance it is much harder to determine exactly what's going on.
- rgarcia 14y agoReally they should just use underscore: @originalStyle = _(@element.style).pick ['top', 'left', 'width', 'height']
- deleted 14y ago[deleted]
- csense 14y agoParent's reduce() example is harder to read than the original, and about the same length. If CoffeeScript had dictionary comprehensions like Python 2.7 / 3.x, we could make the code shorter while preserving and easier to comprehend: @originalStyle = {k : @element.style[k] for k in ['top', 'left', 'width', 'height'} Unfortunately this syntax was proposed and rejected [1]. But with a simple to_hash library function, you can achieve something similar [2]: to_hash = (pairs) -> hash = {} hash[key] = value for [key, value] in pairs hash @originalStyle = to_hash ([k, @element.style[k]] for k in ['top', 'left', 'width', 'height']) [1] https://github.com/jashkenas/coffee-script/issues/467 https://github.com/jashkenas/coffee-script/issues/467 [2] https://gist.github.com/2271874 https://gist.github.com/2271874
- nahname 14y agoIt says they mostly just ported the JS over using JS2Coffee. Maybe they plan to go back through everything and actually convert it over to coffeescript?
- lowboy 14y agoWhy are you bringing reduce() into this? Seems to fly in the face of its common purpose, stated on the MDN page: Apply a function against an accumulator and each value of the array (from left-to-right) as to reduce it to a single value. By doing it this way, you've introduced another symbol for us to consider (hash) and by departing from the standard for loop format, you've made it less idiomatic, which means more people will have to expend mental energy parsing it.
- alinajaf 14y agoJust another datapoint, but contrary to the other comments here I find your reduce example a lot more readable than the original. This could be because it's fairly idiomatic ruby.
- byroot 14y agoFor me this in idiomatic ruby would be: original_style = element.style.only('top', 'left', 'width', 'height') Or if you do not want to rely on ActiveSupport: original_style = Hash[%w(top left width height).map{ |p| [p, element.style[p]] }]
- alinajaf 14y agoWithout ActiveSupport I'd probably type something like this: %w(top left width height).reduce({}) { |memo, pos| memo[position] = @element.style[position] and memo }
- mcantor 14y agoWhat's with the dangling , {} on your first example? I can't tell if it's a typo or a weird wrapped line rendering issue or something. I hope that's not considered idiomatic CoffeeScript.