# coffeescript and angular directive

I feel this is deeply inelegant, but I really want the option of classical inheritance in the future. How could this be written better. FYI animation here is from GSAP if you are curious.

class Modal

@modalElem   = document.getElementById 'modal'
@overlayElem = document.getElementById 'overlay'

@isOpen = false

@getWindowDim : ->
return dim =
height : window.innerWidth or document.documentElement.clientWidth
width  : window.innerHeight or document.documentElement.clientHeight

@openModal : (attrs) ->

unless @isOpen

@isOpen = true

width   = attrs.width
height  = attrs.height
content = attrs.content

wDim    = @getWindowDim()

@modalElem.style.width      = 0
@modalElem.style.height     = 0

@overlayElem.style.display  = 'block'

TweenLite.to @modalElem, 0.75,
width       : width
height      : height
marginTop   : (height/-2)
marginLeft  : (width/-2)
opacity     : 1
ease        : Back.easeOut
@

@closeModal : ->

if @isOpen

@isOpen = false

TweenLite.to @modalElem, 0.33,
width       : 0
height      : 0
marginTop   : 0
marginLeft  : 0
opacity     : 0
ease        : Back.easeOut
onComplete  : =>
@overlayElem.style.display = 'none'
@

modal   = angular.module 'modal'

modal.directive 'openmodal', ->
return (scope, element, attrs) ->
element.bind 'click', ->
Modal.openModal attrs

modal.directive 'closemodal', ->
return (scope, element, attrs) ->
element.bind 'click', ->
Modal.closeModal()


Honestly, I'd recommend against creating a class. You may want classical inheritance in the future, but right now it's not necessary. Premature optimization rarely benefits anyone. Besides, if you do want classical inheritance later, it's pretty trivial to convert the code.

I'd just write it as simply as possible for now, and see what I'll actually need later:

do -> # wrap in an IIFE
# simple local variables
isOpen      = false
modalElem   = null
overlayElem = null
modal       = angular.module 'modal'

modalElem   = document.getElementById 'modal'
overlayElem = document.getElementById 'overlay'

open = (attrs) ->
return if isOpen # return early
isOpen = true

options =
opacity:    1
ease:       Back.easeOut
height:     attrs.height
width:      attrs.width
marginTop:  attrs.height / -2
marginLeft: attrs.width / -2

modalElem.style.width  = 0
modalElem.style.height = 0
overlayElem.style.display = 'block'
TweenLite.to modalElem, 0.75, options

close = ->
return unless isOpen # return early
TweenLite.to modalElem, 0.33,
width:      0
height:     0
marginTop:  0
marginLeft: 0
opacity:    0
ease:       Back.easeOut
onComplete: ->
overlayElem.style.display = 'none'
isOpen = false # moved this here, so it flips when the modal's fully closed

modal.directive 'openmodal', ->
(scope, element, attrs) ->
element.bind 'click', -> open attrs

modal.directive 'closemodal', ->
(scope, element, attrs) ->
element.bind 'click', -> close()


If you're compiling this as a single file with CoffeeScript's default IIFE wrapper, you can skip the do -> and indentation; it'll be added automatically.

If you don't want to take the approach above, here are some tips for your current code:

Using explicit return is only necessary if you want something other than the last evaluated expression to be returned. In you open/close functions, it'd be beneficial to simply return early (like I do in the code above) instead of having a big if branch.

Also, a neat trick:

getWindowDim : ->
return dim =
height : window.innerWidth or document.documentElement.clientWidth
width  : window.innerHeight or document.documentElement.clientHeight


can be written as

getWindowDim : ->
height : window.innerWidth or document.documentElement.clientWidth
width  : window.innerHeight or document.documentElement.clientHeight


The return is implicit, there's no need for a variable assignment, and CoffeeScript will automatically make an object.

• actually it turns out getWindowDim is not consumed at all! In your finished example, please replace height and width with the version from attrs and I will accept your answer. +1 – Fresheyeball Apr 15 '13 at 16:25
• @Fresheyeball Whoops - fixed now – Flambino Apr 15 '13 at 16:33
• It's a trend I don't really like in coffeescript (and ruby). People tends to write as less characters as possible and it alters the readability of the code. I'm not sure that getting rid of the returnreally helps clarity. When the returnkeyword is present, it's clear that the function is meant to return something. It might be personal, but I'm pretty sure I'm not the only one out there. – jackdbernier Feb 11 '14 at 3:00
• @jackdbernier I know what you're saying. But it also depends on how you view the code overall. If you regard it as more functional (as you can with ruby and js/cs), variables and function calls become interchangeable; a function just is its return value - and all functions always return something, even if it's undefined. (Obviously, this looks better in languages - like ruby - where you can leave out the () in calls). Naming is key, though; nouns for pseudo-variable functions, verbs for function functions (i.e. the above should be called windowDimensions without the get prefix) – Flambino Feb 12 '14 at 3:59