The case for multiline signatures
Tags: php, devtips, programming, codingstyle
Published: 2017-10-21 18:18:29

---
title: The case for multiline signatures
published: true
description: if it feels weird, that's because you didn't try long enough
tags: php,devtips,programming,codingstyle
---

Hopefully, [my last blog post](https://dev.to/greg0ire/why-i-keep-pestering-everyone-about-long-lines) convinced you to avoid long lines. The php community has a nice coding style guide called [PSR-2](http://www.php-fig.org/psr/psr-2/) that says the following:

> There MUST NOT be a hard limit on line length; the soft limit MUST be 120 characters; lines SHOULD be 80 characters or less.

Basically this means: do whatever you want, but your editor will warn if you go above 120 characters. The 80 characters limit is probably here to encourage people to add linebreaks even if their line is just 121 chars instead of saying "come on, it's only 1 character".

Some projects take advantage of the limit being soft and disregard the soft limit for method signatures and will write this:

```php
<?php
public function connect(?string $sourceId, $sourceValue, ?string $destId, $destValue = null, string $reference = null, bool $lazy = false, bool $weak = false);
```

rather than this:

```php
<?php
    public function connect(
        ?string $sourceId,
        $sourceValue,
        ?string $destId,
        $destValue = null,
        string $reference = null,
        bool $lazy = false,
        bool $weak = false
    );
```
Behind that decision are many arguments that I'd like to discuss here. Just to be clear though: I'm not suggesting you should always use multiline signatures. In my opinion, they should only be used when the line length exceeds 120 characters.

## Method signatures are not real code

The main argument seems to be that method signatures are not "real code", because they declare things instead of doing things, and should thus not pollute the screen real estate. Except that they describe the interface of your method, and changes to them are very important, and should be reviewed carefully, because that is where most BC-breaks happen. Also, since php 7.0, function signatures are getting far more type hinting than before, which makes them longer, but also more useful, because they describe what type of variables you are dealing with. This is not always obvious unless you use [hungarian notation](https://en.wikipedia.org/wiki/Hungarian_notation), which you absolutely shouldn't.
Having multiline signatures is not only useful for reviewers, but for people that edit that part of the code. If one argument maps to one line, navigating from argument to argument is suddenly way easier than whatever IDE shortcuts you would use otherwise.
Still not convinced? Consider using [code folding](https://en.wikipedia.org/wiki/Code_folding) instead of making the code uglier for everyone else, including and especially reviewers. If this suggestion sounds stupid, maybe it is and the signatures do matter after all?

## Parameter types are already described in the phpdoc

Then remove the phpdoc, unless you wrote something very interesting in it. Forcing people to describe each and every parameter in the phpdoc always results in the same paraphrasing comments in my experience, which has a very bad consequence: people stop reading the comments. Surely you can give the real estate you gave to the phpdoc to a piece of code? Most IDEs display comments in light gray nowadays, probably because of this, but I feel comments should not be mainly aimed at doc-extracting tools or annotation libraries. They should be helpful messages for your team, when your code fails to achieve enough clarity.

## Long method signatures are an alarm signal about having too many arguments

That one is really moot, because if you have a 100000-character long line, it will still be one line, whereas if your screen is filled with a signature, the alarm signal will only be louder, and tools like [phpmd](https://phpmd.org/) will warn about your method having too many arguments.
The first step towards realizing how wrong things are for [this method](https://github.com/magento/magento2/blob/a5be15b42bee194f5030cb516002aab5244fe057/app/code/Magento/Catalog/Model/Product.php#L392) is definitely using a multiline signature:

```php
<?php
    /**
     * Product constructor.
     * @param \Magento\Framework\Model\Context $context
     * @param \Magento\Framework\Registry $registry
     * @param \Magento\Framework\Api\ExtensionAttributesFactory $extensionFactory
     * @param AttributeValueFactory $customAttributeFactory
     * @param \Magento\Store\Model\StoreManagerInterface $storeManager
     * @param \Magento\Catalog\Api\ProductAttributeRepositoryInterface $metadataService
     * @param Product\Url $url
     * @param Product\Link $productLink
     * @param Product\Configuration\Item\OptionFactory $itemOptionFactory
     * @param \Magento\CatalogInventory\Api\Data\StockItemInterfaceFactory $stockItemFactory
     * @param Product\OptionFactory $catalogProductOptionFactory
     * @param Product\Visibility $catalogProductVisibility
     * @param Product\Attribute\Source\Status $catalogProductStatus
     * @param Product\Media\Config $catalogProductMediaConfig
     * @param Product\Type $catalogProductType
     * @param \Magento\Framework\Module\Manager $moduleManager
     * @param \Magento\Catalog\Helper\Product $catalogProduct
     * @param ResourceModel\Product $resource
     * @param ResourceModel\Product\Collection $resourceCollection
     * @param \Magento\Framework\Data\CollectionFactory $collectionFactory
     * @param \Magento\Framework\Filesystem $filesystem
     * @param \Magento\Framework\Indexer\IndexerRegistry $indexerRegistry
     * @param Indexer\Product\Flat\Processor $productFlatIndexerProcessor
     * @param Indexer\Product\Price\Processor $productPriceIndexerProcessor
     * @param Indexer\Product\Eav\Processor $productEavIndexerProcessor
     * @param CategoryRepositoryInterface $categoryRepository
     * @param Product\Image\CacheFactory $imageCacheFactory
     * @param ProductLink\CollectionProvider $entityCollectionProvider
     * @param Product\LinkTypeProvider $linkTypeProvider
     * @param \Magento\Catalog\Api\Data\ProductLinkInterfaceFactory $productLinkFactory
     * @param \Magento\Catalog\Api\Data\ProductLinkExtensionFactory $productLinkExtensionFactory
     * @param EntryConverterPool $mediaGalleryEntryConverterPool
     * @param \Magento\Framework\Api\DataObjectHelper $dataObjectHelper
     * @param \Magento\Framework\Api\ExtensionAttribute\JoinProcessorInterface $joinProcessor
     * @param array $data
     *
     * @SuppressWarnings(PHPMD.ExcessiveParameterList)
     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
     */
    public function __construct(
        \Magento\Framework\Model\Context $context,
        \Magento\Framework\Registry $registry,
        \Magento\Framework\Api\ExtensionAttributesFactory $extensionFactory,
        AttributeValueFactory $customAttributeFactory,
        \Magento\Store\Model\StoreManagerInterface $storeManager,
        \Magento\Catalog\Api\ProductAttributeRepositoryInterface $metadataService,
        Product\Url $url,
        Product\Link $productLink,
        \Magento\Catalog\Model\Product\Configuration\Item\OptionFactory $itemOptionFactory,
        \Magento\CatalogInventory\Api\Data\StockItemInterfaceFactory $stockItemFactory,
        \Magento\Catalog\Model\Product\OptionFactory $catalogProductOptionFactory,
        \Magento\Catalog\Model\Product\Visibility $catalogProductVisibility,
        \Magento\Catalog\Model\Product\Attribute\Source\Status $catalogProductStatus,
        \Magento\Catalog\Model\Product\Media\Config $catalogProductMediaConfig,
        Product\Type $catalogProductType,
        \Magento\Framework\Module\Manager $moduleManager,
        \Magento\Catalog\Helper\Product $catalogProduct,
        \Magento\Catalog\Model\ResourceModel\Product $resource,
        \Magento\Catalog\Model\ResourceModel\Product\Collection $resourceCollection,
        \Magento\Framework\Data\CollectionFactory $collectionFactory,
        \Magento\Framework\Filesystem $filesystem,
        \Magento\Framework\Indexer\IndexerRegistry $indexerRegistry,
        \Magento\Catalog\Model\Indexer\Product\Flat\Processor $productFlatIndexerProcessor,
        \Magento\Catalog\Model\Indexer\Product\Price\Processor $productPriceIndexerProcessor,
        \Magento\Catalog\Model\Indexer\Product\Eav\Processor $productEavIndexerProcessor,
        CategoryRepositoryInterface $categoryRepository,
        Product\Image\CacheFactory $imageCacheFactory,
        \Magento\Catalog\Model\ProductLink\CollectionProvider $entityCollectionProvider,
        \Magento\Catalog\Model\Product\LinkTypeProvider $linkTypeProvider,
        \Magento\Catalog\Api\Data\ProductLinkInterfaceFactory $productLinkFactory,
        \Magento\Catalog\Api\Data\ProductLinkExtensionFactory $productLinkExtensionFactory,
        EntryConverterPool $mediaGalleryEntryConverterPool,
        \Magento\Framework\Api\DataObjectHelper $dataObjectHelper,
        \Magento\Framework\Api\ExtensionAttribute\JoinProcessorInterface $joinProcessor,
        array $data = []
    ) {
```

Besides, with the php 7.0 type hinting, long signatures are going to happen way more often, and you may want to know if the cause is the number of arguments, the length of the type hints, the length of the variable names or a combination of all of these. If you address the real problem behind that, you might end up with more little methods with fewer arguments, which is usually easier to reason about. Don't sweep things under the rug, heed the warnings and design better code. [Symfony's Options Resolver Component](https://github.com/symfony/options-resolver) might help with that if your arguments are actually interchangeable configuration options for something.

## Framework X, which my library uses, forbids multiline signatures

You should strive to make your library framework-agnostic, and segregate it from framework-integrations layers. Style is part of that, use your own, don't let bad decisions spread like as many diseases, especially if you can't help but having many arguments with long names and long type hints because you love Java.

## Switching to multiline signatures will be the source of many conflicts

I'm a maintainer of some libraries that do not use multiline signatures. Guess where a big part of the conflicts happen? That's right, they happen on long lines, which nowadays practically only occur with long signatures. So maybe the conflicts are worth it in the long run.

![long lines are a recipe for git conflicts](https://i.imgflip.com/1xv95r.jpg)

Also, consider that switching to multiline signatures does not have to be done in one-shot: just allow switching to them, and enforce them on newly created methods when they are too long.

## Multiline signatures look weird

Remember switching to PSR-2? I remember, it felt horrible at first to have to write

```
<?php
if (defractulatorIsReady()) {
    defraculate();
}
```

instead of


```
<?php
if (defractulatorIsReady())
{
    defraculate();
}
```

like some of us did for years. But it was also nice to finally have a standard everyone could agree with, so I went with it anyway, and it felt horrible… for about half a day, and that was it. So if multiline signatures feel horrible, please try them a bit longer, instead of clinging to old habits, the language evolves, and so should you!