search

alexpeachey / end_state

A State Machine implementation
MIT License
9 stars 5 forks source link

Extract transition configuration, SOLID-ify transition #19

Closed charlierudolph closed 9 years ago

charlierudolph commented 10 years ago

@alexpeachey @brianvh please review

alexpeachey commented 10 years ago

Overall I like this separation.

I think it's a good start and likely will open up some additional cuts and pieces that we can carve out.

One example might be a class called something (lame default name TransitionConfigurations) that is just a wrapper around the hash that is transition_configurations. It would have a method that basically does the work of __sm_transition_configuration_for. So StateMachine would still have transition_configurations but it would be an instance of this new class instead of a Hash. It seems odd to use this hash and use it like a hash except in an edge case. By wrapping it up in a class it starts to make more sense and the machine doesn't have to understand the specifics of the edge case.

There are probably others that will start to manifest as we look at the new complete code set.

charlierudolph commented 10 years ago

Messages fixed. Will need to add tests for the matchers as they would be broken (changing transitions to transition_configurations)

coveralls commented 10 years ago

Coverage Status

Coverage increased (+0.27%) when pulling 91bcc577909b4b598d4026b435b1878b775b4682 on charlierudolph:cr-simplifyTransition into e67fe0b50b77eeb36c950b0f3e961d1db3da3fcb on Originate:master.

coveralls commented 10 years ago

Coverage Status

Coverage increased (+0.27%) when pulling 91bcc577909b4b598d4026b435b1878b775b4682 on charlierudolph:cr-simplifyTransition into e67fe0b50b77eeb36c950b0f3e961d1db3da3fcb on Originate:master.

coveralls commented 10 years ago

Coverage Status

Coverage increased (+4.86%) when pulling f9fb7b8504d8482427ab64f2c448633c1d2c1a22 on charlierudolph:cr-simplifyTransition into e67fe0b50b77eeb36c950b0f3e961d1db3da3fcb on Originate:master.

coveralls commented 10 years ago

Coverage Status

Coverage increased (+4.86%) when pulling caeb3ceb263b7f200d47b85789af207bb13c0867 on charlierudolph:cr-simplifyTransition into e67fe0b50b77eeb36c950b0f3e961d1db3da3fcb on Originate:master.

charlierudolph commented 10 years ago

@alexpeachey updated. See the code climate diff here

charlierudolph commented 9 years ago

@alexpeachey bump

alexpeachey commented 9 years ago

Thanks @charlierudolph. I think this looks good and at least from what I can see doesn't appear to change the API so I'm merging this. I'm not cutting a new version as this is just a refactoring.