Skip to content

Conversation

taylorjdawson
Copy link
Contributor

@taylorjdawson taylorjdawson commented Aug 1, 2017

In order to improve code reuse, I completely redesigned the way that URLs are built.
I moved the make_url function (now renamed build_url) to the Client class. I thought this to be a better solution as opposed to having the function implemented in every child class.

I believe that this will also make it easier when adding additional functionality.

I tested the new design with the provided example python files and everything ran smoothly.

In order to improve code reuse, I completely redesigned the way that URLs are built.
I moved the make_url function (now renamed build_url) to the Client class. I thought this to be a better solution
as opposed to having the function implemented in every child class. A few changes still need to be made in order
to increase readability.
I updated the token class to comply with the new url build design.
Signed-off-by: Taylor Dawson <taylorjdawson@gmail.com>
To increase code reuse I have moved the build_url method from the child classes and into the parent class, 'Client'.

I also desgined a highly pythonic way of contructing the url.

The children classes are repsonible for setting the values of the api parameters that are located within the parent class.
@corpetty
Copy link
Owner

corpetty commented Aug 1, 2017

It says W.I.P., are all function calls working at the moment?

@taylorjdawson
Copy link
Contributor Author

Yes, I had [W.I.P.] on the first commit but this pull request is complete. I ran all of the example files that you created and everything checked out.

What do you think of this alternative implementation?

@corpetty
Copy link
Owner

corpetty commented Aug 4, 2017

yeah, looks great. I always appreciate code reuse. I never really liked the initial implementation but didn't spend much time thinking about reorganizing it.

Great job.

@corpetty corpetty merged commit 3a41ed0 into corpetty:master Aug 4, 2017
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