argument handling #79

Closed
gjohnson opened this Issue Dec 29, 2012 · 5 comments

Projects

None yet

3 participants

@gjohnson
Collaborator

As I am moving things around a tid-bit, I noticed it would be nice to centralize argument handling, so we don't have this scattered everywhere lol:

if (Array.isArray(msg)) {
  args = msg;
} else {
  for (var i = 0; i < arguments.length; ++i) {
    args[i] = arguments[i];
  }
}

Perhaps have a utils function like this... Orrrr maybe we should be a little more strict about send() for multipart and say that if you want multipart you need to use an array... even though I personally like doing send(a, b, c) over send([a, b, c]).

Meh?

@gjohnson gjohnson closed this Dec 30, 2012
@tj
Owner
tj commented Dec 30, 2012

we have both right now right? (cant remember) I like send(a,b,c) too but both are nice

@gjohnson
Collaborator

Yeah we do have both, not even sure why lol. I would bet that anyone using an array is probably just going to use json and do something like: send({ stuff: [...] }). Perhaps we should only support variadic style multipart, that way we could actually be optimized for single messages and clean up the code a little?

http://jsperf.com/more

@gjohnson gjohnson reopened this Dec 30, 2012
@tj
Owner
tj commented Dec 30, 2012

i think passing an array still makes sense in case you have to built it up, that's easier than applying to the function. Probably wouldn't hurt to chuck it in a util function but those are all way up in the millions ops/s so it probably doesn't affect us much

@tj tj pushed a commit that closed this issue Dec 31, 2012
@gjohnson gjohnson Clean up argument handling
Closes #79.
34a5d69
@tj tj closed this in 34a5d69 Dec 31, 2012
@brycebaril

FYI this tripped me up where I was using send(data) and my data just happened to be an array where I didn't want it to be treated like multipart and had to read the code to see what was going on.

Like the comment above I essentially just changed it to send({stuff: data}) but maybe this is an opportunity for better documentation, or even a different method for explicit multipart send.

@tj
Owner
tj commented Jan 18, 2013

@brycebaril agreed with docs, I think the array as multipart is still useful, mostly because splatting with js sucks but it is more efficient (</overkill>) haha, something we can think about changing though for sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment