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 have such input:

[ { name: 'timezone', value: 'EST' },
  { name: 'interval', value: 'day' },
  { name: 'metrics[]', value: 1},
  { name: 'metrics[]', value: 2} ]

As you may already notice - these are parameters from POST request.

What I need to do is to get such output:

[ { name: 'timezone', value: 'EST' },
  { name: 'interval', value: 'day' },
  { name: 'metrics[]', value: [ 1, 2 ] },
  { name: 'metrics[]', value: [ 1, 2 ] } ]

My code does everything it needs to do, but I'm not sure if it is written in the optimal way, am I overlooking some bugs, etc.

arrayify = (params) ->
  arrayifiedParams = {}

  for param in params
    paramName = param.name

    arrayifiedParams[paramName] = arrayifiedParams[paramName] || []
    arrayifiedParams[paramName].push param.value        

  params.map (param) ->
    paramName = param.name
    if arrayifiedParams[paramName].length > 1
      param.value = arrayifiedParams[paramName]
    param

Here is the corresponding JSFiddle.

share|improve this question
    
For your information, next time you wonder if your question title is good or any concern leave a comment, this will reduce "noise" in your question. You did a good enough job on your question title. Welcome to Code Review! I hope you'll have good review! –  Marc-Andre Jul 8 at 17:49
1  
Do you need to have duplicate objects? –  elclanrs Jul 8 at 19:17
    
@elclanrs nope, I don't. Duplicates should be removed later in the code. –  Shtirlits Jul 8 at 19:21

1 Answer 1

up vote 0 down vote accepted

Your code looks ok overall. There are perhaps a few CoffeeScript features you can make use of, but nothing major.

However, I don't like that the input array is modified in-place (i.e. the function has side-effects). Other code may hold references to the param objects already, so altering them is potentially dangerous.

I'd rather have the function return a new, separate array with the results, leaving the input array untouched.

Here's a version that does that, and also removes duplicate params (I called it coalesce because it sounds fancier):

coalesce = (params) ->
  coalesced = {}

  for param in params
    {name, value} = param
    coalesced[name] or= []
    coalesced[name].push value

  for own name, value of coalesced
    value = value[0] if value.length is 1
    {name, value}
share|improve this answer

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.