Skip to content

Implement Services Interface#4

Open
XedinUnknown wants to merge 4 commits into0.1.xfrom
task/extract-interfaces
Open

Implement Services Interface#4
XedinUnknown wants to merge 4 commits into0.1.xfrom
task/extract-interfaces

Conversation

@XedinUnknown
Copy link
Member

@XedinUnknown XedinUnknown requested a review from mecha March 16, 2021 07:02
@XedinUnknown XedinUnknown self-assigned this Mar 16, 2021
@XedinUnknown XedinUnknown added the enhancement New feature or request label Mar 16, 2021
@XedinUnknown XedinUnknown added this to the v0.1 milestone Mar 16, 2021
Copy link
Member

@mecha mecha left a comment

Choose a reason for hiding this comment

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

Rather than having Factory and Extension implement ServiceInterface, why not have the Service base class implement the interface and remove the duplicate abstract __invoke method? That way we avoid any signature conflicts between the Service class and the interface.

@XedinUnknown
Copy link
Member Author

why not have the Service base class implement the interface

Because it is abstract. I don't want to impose an interface on an abstract class.

remove the duplicate abstract __invoke method

Missed that, thanks!

@mecha
Copy link
Member

mecha commented Mar 16, 2021

Because it is abstract. I don't want to impose an interface on an abstract class.

I don't understand. The purpose of the class is to reduce duplicate code between Factory and Extension. It's not really "imposing" if it's by design. But if you have strong feelings against this, I won't argue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants