[RFC] Remove "*.class" parameters

[pamil]

We have two types of *.class parameters:

• the ones from ResourceBundle: sylius.model.resource.class etc.
• the ones from any other services

In this RFC I would like to address the second ones, reasoning behind removing these is similar to Symfony (citation from initial symfony/symfony#11881):

All services in core define their classes as parameters. The convention is to create a foo.class parameter for the class of the foo service. This approach comes with several problems:

The parameters are dumped like any other one in the compiled class and it makes the file larger for no good reasons (read: it slows down your app for free).

When someone wants to change a service, just changing the class name is rarely enough: the new service probably have some different constructor arguments, ... Or put another way, overriding a service by just changing the class name of a service is a very rare use case.

Of course, if several bundles change the value of such a parameter, the "last" definition wins. Classic inheritance vs composition problem.

For all these reasons, I like to remove all those parameters for Symfony 3.0.

[tristanbes]

👍 I do agree with you @pamil

[pjedrzejewski]

👍 for services not related to the Resources!

[peteward]

Or put another way, overriding a service by just changing the class name of a service is a very rare use case.

Not that rare in our case Mr. Fabpot, it's actually very convenient:

    sylius.availability_checker.default.class: Reiss\Component\Inventory\AvailabilityChecker
    sylius.email_manager.order.class: Reiss\Bundle\StoreBundle\EmailManager\OrderEmailManager
    sylius.callback.order_payment.class: Reiss\Bundle\StoreBundle\StateMachine\Callback\OrderPaymentCallback
    sylius.checkout_scenario.class: Reiss\Bundle\StoreBundle\Checkout\CheckoutProcessScenario
    sylius.checkout_step.finalize.class: Reiss\Bundle\StoreBundle\Checkout\Step\FinalizeStep
    sylius.checkout_step.security.class: Reiss\Bundle\StoreBundle\Checkout\Step\SecurityStep
    sylius.context.currency.class: Reiss\Bundle\StoreBundle\Regionalization\Context\CurrencyContext
    sylius.controller.backend.dashboard.class: Reiss\Bundle\StoreBundle\Controller\DashboardController
    sylius.controller.backend.form.class: Reiss\WebBundle\Controller\Backend\FormController
    sylius.controller.checkout.class: Reiss\Bundle\StoreBundle\Controller\CheckoutController
    sylius.controller.frontend.account.address.class: Reiss\WebBundle\Controller\Frontend\Account\AddressController
    sylius.controller.frontend.homepage.class: Reiss\WebBundle\Controller\HomePageController
    sylius.custom_factory.adjustment.class: Reiss\Component\Order\Factory\AdjustmentFactory
    sylius.email_sender.adapter.swiftmailer.class: Reiss\Bundle\StoreBundle\Mailer\Sender\Adapter\MandrillSenderAdapter
    sylius.form.type.order_filter.class: Reiss\Bundle\StoreBundle\Form\Type\Filter\OrderFilterType
    sylius.form.type.promotion_action.add_product_configuration.class: Reiss\Bundle\StoreBundle\Form\Type\Action\AddProductConfigurationType
    sylius.form.type.promotion_action.fixed_discount_configuration.class: Reiss\Bundle\StoreBundle\Form\Type\Action\FixedDiscountConfigurationType
    sylius.form.type.promotion_action.percentage_discount_configuration.class: Reiss\Bundle\StoreBundle\Form\Type\Action\PercentageDiscountConfigurationType
    sylius.form.type.promotion_rule.contains_product_configuration.class: Reiss\Bundle\StoreBundle\Form\Type\Rule\ContainsProductConfigurationType
    sylius.generator.class: Reiss\Bundle\StoreBundle\Routing\ReissAwareGenerator
    sylius.listener.mailer.class: Reiss\Bundle\StoreBundle\EventListener\MailerListener
    sylius.listener.order_inventory.class: Reiss\Bundle\StoreBundle\EventListener\OrderInventoryListener
    sylius.listener.order_payment.class: Reiss\Bundle\StoreBundle\EventListener\OrderPaymentListener
    sylius.listener.user_mailer_listener.class: Reiss\Bundle\StoreBundle\EventListener\User\MailerListener
    sylius.model.customer.class: Reiss\Component\User\Model\Customer
    sylius.order_processing.prices_recalculator.class: Reiss\Bundle\StoreBundle\Order\OrderProcessing\PricesRecalculator
    sylius.order_processing.state_resolver.class: Reiss\Bundle\StoreBundle\Order\OrderProcessing\StateResolver
    sylius.process.coordinator.class: Reiss\Bundle\StoreBundle\Process\Coordinator\Coordinator
    sylius.process.context.class: Reiss\Bundle\StoreBundle\Process\ProcessContext
    sylius.promotion_action.fixed_discount.class: Reiss\Bundle\StoreBundle\Promotion\Action\FixedDiscountAction
    sylius.promotion_action.percentage_discount.class: Reiss\Bundle\StoreBundle\Promotion\Action\PercentageDiscountAction
    sylius.route_provider.class: Reiss\Bundle\StoreBundle\Routing\RouteProvider
    sylius.sequence.model.class: Sylius\Component\Sequence\Model\Sequence
    sylius.shipping_methods_resolver.class: Reiss\Bundle\StoreBundle\Shipping\Resolver\MethodsResolver

If you decide to do this, it would be great if you wait until after 0.18.

[peteward]

Or... I would actually prefer the performance of a smaller container so go ahead, we can cope with it easily enough 👍

[pjedrzejewski]

For some classes, I think it still makes sense, most of form types, etc. We should make a list and decide what is good for DX.

[tristanbes]

@peteward why don't you use service decorator ? (http://symfony.com/doc/current/components/dependency_injection/advanced.html#decorating-services)

[peteward]

I have a love/hate relationship with decorators. Sometimes it's a great pattern, other times it causes mass confusing and limitations (as I've experienced trying to work with some of the Sylius decoration).

But yes, we use it in some cases, sometimes we override the entire service definition (if we need to use different arguments), but sometimes it's just really easy and convenient to override a single class parameter.

Also decoration only works if you're keeping the underlying behaviour the same, which in some cases we aren't.

[ghost]

I had to specifically override a class due to #4959 . It would have been a lot harder without the class names. If Sylius tries harder with BC issues, then I won't mind so much though.

[pamil]

I've just sent the PRs removing all obvious usages of *.class parameters. The following bundles were skip:

• InstallerBundle (to be merged into CoreBundle), [CoreBundle][InstallerBundle] Moved InstallerBundle innards into CoreBundle #6203
• ProductBundle / VariationBundle (VariationBundle is going to be merged into ProductBundle), [Product] [Variation] Merge Variation into Product #6150
• OrderBundle / CartBundle (CartBundle is going to be merged into OrderBundle), [Order] Remove .class parameters #6268
• ResourceBundle (needs more time :))

[pamil]

Created issue #6281 specific for ResourceBundle, closing this one for clarity.