feat(imagick): add antialias option to ImagickImageBackEnd (#209)#235
feat(imagick): add antialias option to ImagickImageBackEnd (#209)#235
Conversation
Add a $antialias constructor parameter (default true) to allow disabling antialiasing for sharper images, smaller file sizes, and better nearest-neighbor scalability. Closes #209 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #235 +/- ##
============================================
- Coverage 71.81% 71.79% -0.03%
- Complexity 995 996 +1
============================================
Files 49 49
Lines 3151 3155 +4
============================================
+ Hits 2263 2265 +2
- Misses 888 890 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks for adding the antialias mode! Since this hasn't shipped yet, I’d like to suggest a different approach for the implementation. Currently, adding settings to the constructor creates a long sequence of Flag Arguments. This pattern makes the API harder to read—for example, in I’d suggest one of the following two patterns to keep the API clean and extensible: Option A: Fluent "With" MethodsKeep the constructor minimal for required dependencies and use "wither" methods for optional configurations. This is self-documenting and keeps the usage clear. public function withAntialias($antialias): self {
$clone = clone $this;
$clone->antialias = $antialias;
return $clone;
}
// Usage: $backend = (new ImagickImageBackEnd())->withAntialias(false);Option B: Configuration ObjectIf we expect the list of settings to grow (DPI, rendering intents, etc.), passing a dedicated options object is a very scalable way to handle configuration. final class ImagickOptions {
public function __construct(
public readonly string $format = 'png',
public readonly int $quality = 100,
public readonly bool $antialias = true,
) {}
}
// Usage: $backend = new ImagickImageBackEnd(new ImagickOptions(antialias: false));Option A seems interesting, though for codebase consistency, other such "chained setters" should eventually be implemented for the other settings as well. Option B may seem redundant compared to just using the constructor with named parameters, but it adds an abstraction layer that is significantly more useful for future maintenance and evolution (e.g., standardizing options across different backends). Both of these methods introduce new paradigms to the codebase, thus they should be carefully considered. However, I'd like to take the opportunity that this "parameter hell" hasn't been released yet to discard it in favor of a more descriptive approach. What do you think? |
It's actually in 3.1.0 already. We could consider a different approach for 4.x, but not too sure about either approach there. |
|
Although to be fair, fluent setters could be added on top of the current constructor method. |
|
I noticed after commenting that the change had already been published. That’s fine — with only three arguments, the current constructor remains acceptable. My proposals are simply ideas to keep in reserve should additional options be introduced in the future. |
Add a $antialias constructor parameter (default true) to allow disabling antialiasing for sharper images, smaller file sizes, and better nearest-neighbor scalability.
Closes #209