Skip to content

react: Add JSON test data #358

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 6, 2016
Merged

react: Add JSON test data #358

merged 2 commits into from
Sep 6, 2016

Conversation

petertseng
Copy link
Member

Existing implementations:

And an upcoming implementation (that uses these tests):

The tests were a union of the existing tests plus a few extras to
provide more gradual steps in the early stages (when still setting up
the input and compute cells).

Closes https://github.com/exercism/todo/issues/135

@petertseng
Copy link
Member Author

I don't have much in the way of verification here, though I did make sure I didn't mess up the schemas:

require 'json'

js = JSON.parse(File.read('react.json'))

def verify(h)
  h.each { |k, v|
    raise "Nope, #{k} has multiple schemas: #{v}" if v.size > 1
    keys, times = v.first
    puts '%22s: %2d times, keys %s' % [k, times, keys]
  }

end

puts 'Cells:'
verify(js['cases'].each_with_object(Hash.new { |h, k| h[k] = Hash.new(0) }) { |c, h|
  c['cells'].each { |cell|
    ks = cell.keys
    raise "#{cell} has no name" unless ks.delete('name')
    raise "#{cell} has no type" unless ks.delete('type')
    h[cell['type']][ks] += 1
  }
})

puts 'Operations:'
verify(js['cases'].each_with_object(Hash.new { |h, k| h[k] = Hash.new(0) }) { |c, h|
  c['operations'].each { |op|
    ks = op.keys
    raise "#{op} has no type" unless ks.delete('type')
    h[op['type']][ks] += 1
  }
})
Cells:
                 input: 13 times, keys ["initial_value"]
               compute: 17 times, keys ["inputs", "compute_function"]
Operations:
     expect_cell_value:  8 times, keys ["cell", "value"]
             set_value: 14 times, keys ["cell", "value"]
          add_callback:  9 times, keys ["cell", "name"]
expect_callback_values: 10 times, keys ["callback", "values"]
       remove_callback:  4 times, keys ["cell", "name"]

@petertseng
Copy link
Member Author

petertseng commented Sep 3, 2016

Now verifying callbacks and cell names, fixed all bad names caught by this:

require 'json'

cases = JSON.parse(File.read('react.json'))['cases']

def verify(h)
  h.each { |k, v|
    raise "Nope, #{k} has multiple schemas: #{v}" if v.size > 1
    keys, times = v.first
    puts '%22s: %2d times, keys %s' % [k, times, keys]
  }

end

puts 'Cells:'
verify(cases.each_with_object(Hash.new { |h, k| h[k] = Hash.new(0) }) { |c, h|
  c['cells'].each { |cell|
    ks = cell.keys
    raise "#{cell} has no name" unless ks.delete('name')
    raise "#{cell} has no type" unless ks.delete('type')
    h[cell['type']][ks] += 1
  }
})

puts 'Operations:'
verify(cases.each_with_object(Hash.new { |h, k| h[k] = Hash.new(0) }) { |c, h|
  c['operations'].each { |op|
    ks = op.keys
    raise "#{op} has no type" unless ks.delete('type')
    h[op['type']][ks] += 1
  }
})

cases.each { |c|
  cells = []

  c['cells'].each { |cell|
    cells << cell['name']
    raise "#{cell} has illegal input: #{cell['inputs']} vs #{cells}" unless (cell['inputs'] || []).all? { |i| cells.include?(i) }
  }

  active_callbacks = []
  all_callbacks = []

  c['operations'].each { |op|
    if %w(expect_cell_value set_value add_callback remove_callback).include?(op['type'])
      raise "#{op} has illegal cell: #{op['cell']} vs #{cells}" unless cells.include?(op['cell'])
    end
    if op['type'] == 'add_callback'
      all_callbacks << op['name']
      active_callbacks << op['name']
    end
    active_callbacks.delete(op['name']) if op['type'] == 'remove_callback'
    if op['type'] == 'expect_callback_values' && !all_callbacks.include?(op['callback'])
      raise "#{op} wants invalid callback, #{op['callback']} vs #{active_callbacks}"
    end
  }
}

I think the only thing left unverified by this are the actual values - if I actually went and verified that automatically I'd have to simulate a reactor from this JSON so I probably will have to leave that for manual verification

Existing implementations:

* https://github.com/exercism/xfsharp/blob/master/exercises/react/ReactTest.fs
* https://github.com/exercism/xgo/blob/master/exercises/react/react_test.go
* https://github.com/exercism/xnim/blob/master/react/react_test.nim

And an upcoming implementation (that uses these tests):

* exercism/rust#191

The tests were a union of the existing tests plus a few extras to
provide more gradual steps in the early stages (when still setting up
the input and compute cells).

Closes https://github.com/exercism/todo/issues/135
@petertseng
Copy link
Member Author

petertseng commented Sep 3, 2016

Additional note: Go was the first language with this exercise.

Go limited compute cells to one-input and two-input compute cells, with separate createCompute1 and createCompute2 functions. The advantage of this in a statically type checked language is that you can let the compiler ensure that you don't pass a two-input function to a one-input compute cell, or other similar errors.

All subsequent implementing languages have simply had a single create_compute that takes a list of input cells, and the compute function takes a single input (a list of input values). The benefit is obviously far more flexibility in arity, the downside is that there is no way to check whether the arities match up.

I'm not sure whether to call out these two options in the JSON file.

Edit: Added as separate commit.

@IanWhitney
Copy link
Contributor

Based on what I've seen in the Rust implementation, this approach seems ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants