Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Imagine more hackable, remove final keywords from classes and functions declaration #339

Closed

Conversation

romainneutron
Copy link
Collaborator

This solves #162.

After thinking about it, I think we should open the interface so Imagine would be more hackable. I don't think classes should be extended, anyway, if some use case require it, let's give developers the ability to do so.

I propose to do it now as it would solve an issue toward version 1.0

@Richtermeister
Copy link
Contributor

Big 👍 I am just running into this issue because I need to mock Imagine\Imagick\Image via phpspec and can't do so. Opening this up would be great, thank you.

@avalanche123
Copy link
Collaborator

You should mock the interface

@avalanche123
Copy link
Collaborator

I'm +0 on this change

@Richtermeister
Copy link
Contributor

@avalanche123 I'm referring to the Imagick-specific Image implementation, there does not seem to be an interface for it (unless I'm overlooking something).

@stof
Copy link
Contributor

stof commented Jun 24, 2014

there is an ImageInterface with the methods of the Image. All drivers have the same methods in their Image implementation

@stof
Copy link
Contributor

stof commented Jun 24, 2014

IMO, a much better fix would be to provide needed extension points through composition in the library rather than opening for inheritance.
If you look at Behat 3 for instance, we managed to have a fully-extensible library while having final class quite everywhere

@Richtermeister
Copy link
Contributor

@stof This method here: https://github.com/avalanche123/Imagine/blob/develop/lib/Imagine/Imagick/Image.php#L95 is not in the Interface, it is unique to the ImagickImage.

I have a filter which only works on Imagick, not on GD, and I am using this method to access the underlying Imagick instance. This work great, but I cannot test it.

@mlocati
Copy link
Collaborator

mlocati commented Aug 28, 2018

I'm closing this because too many changes occurred. Let's continue the discussion in #162

@mlocati mlocati closed this Aug 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants