Skip to content

Change attributes visibility to allow proper inheritance of this clas…#9

Open
ruudy-es wants to merge 2 commits intof3ath:masterfrom
ruudy-es:master
Open

Change attributes visibility to allow proper inheritance of this clas…#9
ruudy-es wants to merge 2 commits intof3ath:masterfrom
ruudy-es:master

Conversation

@ruudy-es
Copy link

Some endpoints on AppNexus API do not return an object as a result, i just found one critical endpoint for what im currently working on that return a redirect and a url i have to use on the header.

There are several possibilities to fix that, but at any case i need to extend AppNexusClient and also implement my own HttpClient to be injected on the AppNexusClient construct.

…s and the possibility to manage the construct of it

Increase gitignore coverage

deleted idea
.gitignore Outdated
.idea*

#OS
.DS_Store
Copy link
Owner

Choose a reason for hiding this comment

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

Please do not add environment-specific things to the project ignore file. Stuff like .DS_Store belongs in your local ~/.gitignore

@f3ath
Copy link
Owner

f3ath commented Jun 23, 2017

Hi @ruudy-es Thanks for opening the PR. I'd like to learn more about the issue you're having. What is this endpoint? Can the problem be solved if we extract HttpClient interface and then you'll be able to create a wrapper class over it to handle redirect responses properly? In general, I'd like to use composition over inheritance to stick to the Open-Close Principle.

@ruudy-es
Copy link
Author

ruudy-es commented Jun 24, 2017

Hi @f3ath, thanks for your fast answer :D.

I found the problem here: https://wiki.appnexus.com/display/api/Log-Level+Data+Service#Log-LevelDataService-Step2.Requestthedownloadlocationforafeed

I get your point but i think at least http attribute inside AppNexusClient should be protected, i want to extend this class and to strictly has to pass your HttpClient class to the constructor limit the extensibility i have in mind.

An Interface of the HttpClient could solve the problem, but ill fully detached from your HttpClient logic, and ill probably wants to keep your "call" functionality and implement extra ones, that way my project will be allowed to keep in maintenance when you update or fix anything on your bundle.

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