current community

your communities

more stack exchange communities

Stack Exchange
tour help
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.
up vote 2 down vote favorite

I made an object wrapper/parameter injector and I hope it will be useful, I made it for a project I am working on but then decided to try polishing it a bit.

The use case is for wrapping objects such that you can inject parameters into their methods by calling the wrapper as a function. I was heavily inspired by AngularJS and their use of parameter injection to almost create a DSL with their framework.

I'm looking for answers to these questions: Is this a well-designed module? Is my code legible? Am I over looking something that might lead to bugs later? Is something like this safe to use?

Answers to questions I didn't ask are also welcome.

I'm including a link to the repo here. The code follows

###
negotiator.js

A small tool for proxying objects behind a wrapper that can inject
parameters into their methods.
###


# Extract parameter names from a function,
# position is given by position this array.
if module
  _ = require('underscore');
else
  _ = window._

utils = {};

utils.parameterNames = (func) ->

  funcArgs = func.toString().split('(')[1].split(')')[0].split(',')

  return (k.trim() for k in funcArgs)


# Inject parameters from a context object into a function.
# Passed parameters are supplied by the 'params' argument.
utils.injectAndApply =(func, parameters, context, target) ->

  signature = utils.parameterNames func
  parameters = [] if _.isEmpty(parameters)

  for position, name of signature
    parameters[position] = context[name] if context[name]?

  parameters.length = signature.length;
  return func.apply target ? {}, parameters


# Constructor for proxy object.
utils.Proxy = (real) ->

  @$real = real
  self = this

  for key, method of real when typeof method is 'function'
    do (method, key) ->
      self[key] = ->
        utils.injectAndApply method, arguments, @$context ? {}, @$real

  return this


# Build a context object from an arguments list.
utils.buildContextFromParams = (func, parameters) ->
  signature = utils.parameterNames func
  context = {}
  for key, value of parameters
    context[signature[key]] = value

  return context


# This is the circular wrapper that returns a version of itself with
# a set context.
utils.innerWrapper = (proxy, templateFunction, parameters) ->
  wrapper = ->
    utils.innerWrapper(proxy,templateFunction,arguments);

  context = utils.buildContextFromParams templateFunction, parameters 
  context.$real = proxy.$real
  context.$wrapper = wrapper
  context.$proxy =  proxy
  context.$context = context;
  utils.injectAndApply templateFunction, parameters, context, proxy
  wrapper.$context = context;

  wrapper.__proto__ = proxy;

  return wrapper

# Returns a function that wraps object.
# not intended to be called as a constructor
utils.makeWrapper = (real, templateFunction) ->
  proxy = new utils.Proxy real

  return utils.innerWrapper proxy, templateFunction, []

negotiator = utils.makeWrapper
negotiator.utils = utils;

if module
  module.exports = negotiator
else
  window.negotiator = negotiator

return
share|improve this question
    
I don't know if this is useful advice but you could use some common CoffeeScript conventions like implicit returns, and => to bind a function to the current value of this. You also don't need to end lines with semicolons. I highlighted some of these with #CR: in this Gist: gist.github.com/wildlyinaccurate/5021863 –  WildlyInaccurate Feb 23 '13 at 23:42
add comment

1 Answer

up vote 2 down vote accepted

Looks quite good. Nevertheless here are some thoughts:

Parameter Names

You might get problems when calling utils.parameterNames without arguments. E.g. func.toString() would fail if func is undefined. Moreover you'll get an array with one entry (['']) if you pass a function with no arguments (utils.parameterNames(->)) where you might expect an empty array. You could change your implementation to this:

utils.parameterNames = (func=->) ->
  funcArgs = func.toString().split('(')[1].split(')')[0].split(',')
  (t for k in funcArgs when (t=k.trim()) isnt '')

Here is an other implementation (source) using an regular expression

getArgumentNames = (fn=->) ->
  args = fn.toString().match ///
    function # start with 'function'
    [^(]* # any character but not '('
    \( # open bracket = '(' character
    ([^)]*) # any character but not ')'
    \) # close bracket = ')' character
  ///
return [] if not args? or (args.length < 2)
args = args[1]
args = args.split /\s*,\s*/
(a for a in args when a.trim() isnt '')

We could also write s.th. like this:

fnRgx = ///
  function # start with 'function'
  [^(]* # any character but not '('
  \( # open bracket = '(' character
  ([^)]*) # any character but not ')'
  \) # close bracket = ')' character
///

argRgx = /([^\s,]+)/g

parameterNames = (fn) ->
  (fn?.toString().match(fnRegex)?[1] or '').match(argRgx) or []

The performance depend on the browser but the last one seems to be pretty fast (see benchmarks)

Expressions

In CoffeeScript every thing is an expression so you can turn this

if module
  _ = require('underscore');
else
  _ = window._

into this

_ = if module then require('underscore') else window._

Iteration

This:

for position, name of signature
  parameters[position] = context[name] if context[name]?

could also be written in that way:

for position, name of signature when context[name]?
  parameters[position] = context[name]

or even:

parameters[position] = context[name] for position, name of signature when context[name]?

It compiles all to the same JS. I'd prefer the second variant.

share|improve this answer
add comment

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.