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 Loader #65

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

angelsanzn
Copy link
Contributor

Again, some refactors I noticed while reading the code.

This mostly boils down to:

  • making the methods more uniform
  • making explicit the division between classes/methods and domain concepts (examples, example groups, hooks, helper methods...)

The commit messages are more detailed.

Some of this might be a matter of personal taste (especially the naming), so I'm open to changes ;)

@angelsanzn angelsanzn changed the title Refactor loader Refactor Loader Dec 7, 2015
Ángel Sanz added 20 commits February 5, 2016 01:45
The subject of an example group is either a class object
or a string. The latter is only the case when the call to
`getattr` returns the default vaule (the third argument
given in the call). Hence we can chop the string just there
instead of checking it later.
to `_methods_in` for consistency with similar methods
_example_groups_for -> _top_level_classes_in: at the beginning,
top-level classes are extracted, from either module objects or
class objects themselves (in the case of nested examples). But
they are not example groups yet.

_is_example_group -> _is_name_of_example_group: the parameter
is expected to be a string, hence it's not really a candidate
for an example group. It's a name of something, and the method
determines whether that something is an example group.

_is_hook -> _is_name_of_hook: same as above.
…he code

The fact that these classes are nested within klass is reflected
in the call to _top_level_classes_in(klass).
This gives that method more symmetry with respect to the all other
operations in the class.

This also extracts another method from the extracted method, for more
clearly stating what a helper method is.
…xecution_context`

and bury Python version check
This delimits the boundary of the 'dichotomy' between classes
and example groups, and makes all related query methods in the
class have a similar signature.
The name of the method being called is more descriptive.
This also makes it a bit more explicit that instantiating
an ExampleGroup doesn't add examples, hooks or nested examples
to it.
This makes the remaining ones match the signatures of the previously
refactored ones: since we're looking at the name, receive only the name,
and make that clear in the name of the method.
This makes clearer that this method doesn't return `Example`s
but methods, which it uses to create the `Example`s.
This is unnecessary because _create_bare_example_group only takes
two parameters and it's almost always called with an explicit
second parameter.
This is the way the other methods querying names are naming their parameters.
The fact that it's the name of a class is irrelvant to
`_is_name_of_example_group`.
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.

1 participant