Skip to content
This repository has been archived by the owner on Oct 22, 2019. It is now read-only.

Add Interfaces to Boundaries #61

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rdohms
Copy link
Contributor

@rdohms rdohms commented Nov 17, 2017

The registry and the rederer are classes that are usually injected by
users into their own code, having interfaces for them allow users to
mock based on the API and not on the concret implementation which avoids
a common code smell. This also allows better custom implementations if
users so desire.

The registry and the rederer are classes that are usually injected by
users into their own code, having interfaces for them allow users to
mock based on the API and not on the concret implementation which avoids
a common code smell. This also allows better custom implementations if
users so desire.
@bracki
Copy link
Contributor

bracki commented Nov 23, 2017

Thanks for your contribution!
Can you outline your use case? What testing library are you using? Shouldn't the InMemory() adapter be sufficient? Also the renderer just takes a bunch of metrics and outputs text, I don't see how having an interface for this helps.

@rdohms
Copy link
Contributor Author

rdohms commented Nov 23, 2017

@bracki testing just a side effect, the issue here is more about package design and decoupling.

Part of the reason is that we design our code with SOLID in mind and thus we try to keep to the "D" for Dependency Inversion, which essentially means rely on abstractions not concrete implementations. (more here)

Further more Uncle Bob's Package coupling concepts, specifically SDP and SAP talk extensively about package stability via the DIP. You can read more here

I mention testing as this is where we can clearly observe these dependencies. While trying to test the classes that consume this library I need to make a few mocks and since we avoid mocking concrete classes its easy to observe where interfaces would give us the more stable code to work against.

@rdohms
Copy link
Contributor Author

rdohms commented Dec 6, 2017

@bracki any news on this?

@NoelDavies
Copy link

This project is dead, but I'm maintaining it under my employer - https://github.com/endclothing/prometheus_client_php. Feel free to submit the PR there.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants