Skip to content
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

Refactor to contracts engine #141

Merged
merged 13 commits into from
May 10, 2015

Conversation

waterlink
Copy link
Collaborator

Allows to be as non-intrusive as possible, moving all the functionality effectively to only one method .__contracts_engine (which has rich object(s) inside).

Still needs some cleanup:

  • refactor MethodHandler#handle.
  • remove Eigenclass.
  • remove include Contracts::Module since now they get all the needed stuff properly.
  • try to reduce amount of public methods on Engine. Or at least mark some of them as _pseudo_private. There is a probability, that some of them could be moved to protected (interaction between Engine and EigenclassEngine).
  • add documentation in new code

closes #138

@waterlink
Copy link
Collaborator Author

@egonSchiele @sfcgeorge This one is not finished yet, but can you take a look and tell if you like the direction where this PR is going?

end

def set_eigenclass_owner
Engine.fetch_from(eigenclass).owner_class = target
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just eigenclass_engine here

@betamatt
Copy link
Contributor

❤️

@egonSchiele
Copy link
Owner

Works for me. Also happy that Contracts::Module can be removed! Since you are working on this, could you please add comments on all the methods? That will make it easy to modify this code next time...it is pretty complex.

@alex-fedorov alex-fedorov changed the title [WIP] Refactor to contracts engine Refactor to contracts engine May 4, 2015
@waterlink waterlink force-pushed the non_intrusive_contracts_v2 branch from 46975fc to ea19e3b Compare May 4, 2015 16:40
@waterlink
Copy link
Collaborator Author

Finally green on CI. @egonSchiele can you take a look?

@egonSchiele
Copy link
Owner

Looks great. Thank you for adding all the comments!

egonSchiele added a commit that referenced this pull request May 10, 2015
@egonSchiele egonSchiele merged commit da13531 into egonSchiele:master May 10, 2015
@egonSchiele
Copy link
Owner

Weirdly, the change did make contracts a little slower.

Old:

                                     user     system      total        real
testing add                      0.430000   0.000000   0.430000 (  0.435320)
testing contracts add            3.540000   0.000000   3.540000 (  3.547737)

New:

                                     user     system      total        real
testing add                      0.430000   0.000000   0.430000 (  0.435508)
testing contracts add            3.950000   0.000000   3.950000 (  3.960544)

Not a meaningful slowdown.

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.

Decorator implementation conflicts with Draper
3 participants