ImageStylesMappingService have to many responsability, can you split collecting data and rendering ?

Comments

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Hello,

Thanks for you suggestion. It is in the task #2443757: Possible improvements, I have a work in progress patch here #2443757-8: Possible improvements.

I want to make a first separation in the report generation by getting a plugin architecture where each plugin had a column to the report. For example a plugin for image style, a plugin for responsive image mapping.

ilmir’s picture

ilmir commented

I notice #2443757 where you split mapping responsability, but i think rendering of the data should remain in the controller . Your service should only return data collected in form of object or array.

  • Grimreaper committed 64a041e on 8.x-1.x 
    Issue #2510212 by Grimreaper: Fix html_tag renderable array.
  • Grimreaper committed b5d3390 on 8.x-1.x
    Issue #2510212 by ilmir, Grimreaper: Improve SingleResponsability
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Status: Active » Fixed

Hello,

During the split I realised that the renderable array for html_tag was not good so I fixed it.

And then I moved the renderable functions for the service to the controller.

But there is something that disturbs me. I have the feeling that the reports are less easily overridable.

Maybe with the plugin architecture, I will improve it at this moment.

Feel free to reopen if you think there is a better way to do it.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.