Skip to content

Conversation

shin-
Copy link
Contributor

@shin- shin- commented Jun 24, 2015

No description provided.

@shin- shin- added this to the 1.3.0 milestone Jun 24, 2015
@mpetazzoni
Copy link
Contributor

Unless you expect to have other subclasses of ClientBase, I don't really see the point.

If readability of the Client class is a concern, then yes some utility methods need to be extract, but in a way that doesn't depend on instance state (aka by taking what they need as arguments). Then you can extract those methods into one or more side helper modules.

@shin-
Copy link
Contributor Author

shin- commented Jun 30, 2015

I agree that the end goal should be to have most of those outside of the class, but I was considering this as a first step in separating API code from internal code. I strongly believe it's still better than the current state of the code and it's relatively low effort.

@shin- shin- force-pushed the clientbase_extraction branch from 0862265 to abef716 Compare June 30, 2015 22:27
@shin- shin- force-pushed the clientbase_extraction branch from abef716 to 4f89ca7 Compare June 30, 2015 22:27
mpetazzoni added a commit that referenced this pull request Jul 2, 2015
@mpetazzoni mpetazzoni merged commit f3453aa into master Jul 2, 2015
@mpetazzoni
Copy link
Contributor

Alright, let's work from there.

@mpetazzoni mpetazzoni deleted the clientbase_extraction branch July 2, 2015 00:56
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