Skip to content

0016 - Use variadic to type-hint PHP arrays#24

Open
Amoifr wants to merge 12 commits intoPrestaShop:masterfrom
Amoifr:patch-1
Open

0016 - Use variadic to type-hint PHP arrays#24
Amoifr wants to merge 12 commits intoPrestaShop:masterfrom
Amoifr:patch-1

Conversation

@Amoifr
Copy link
Copy Markdown

@Amoifr Amoifr commented Jan 7, 2022

Questions Answers
Description? New ADR for Variadic
Type? improvement
BC breaks? no
Deprecations? no
Fixed ticket?
How to test?
Possible impacts?

@Amoifr Amoifr changed the title Create Variadic usage.md 0016 - Create Variadic usage.md Jan 7, 2022
@Amoifr Amoifr changed the title 0016 - Create Variadic usage.md 0016 - Create Variadic usage Jan 7, 2022
Co-authored-by: SZCZUPAK Valentin <valentin.szczupak@prestashop.com>
@NeOMakinG
Copy link
Copy Markdown

Even if it's PHP and I'm a front-end developer, I have to agree with this, no kind of types like in TS with Array<Products> in PHP?

@Amoifr
Copy link
Copy Markdown
Author

Amoifr commented Jan 7, 2022

@NeOMakinG The best way is to pass a DTO as a parameter, but this is a workaround if you just have one dimension.

Copy link
Copy Markdown
Contributor

@matks matks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some improvements needed about the text and details but I approve the idea

@matks matks changed the title 0016 - Create Variadic usage 0016 - Use variadic to type-hint PHP arrays Jan 7, 2022
@matks
Copy link
Copy Markdown
Contributor

matks commented Jan 7, 2022

ADR added to README table https://github.com/PrestaShop/ADR/blob/master/README.md

Amoifr and others added 3 commits January 8, 2022 08:37
Co-authored-by: Mathieu Ferment <mathieu.ferment@prestashop.com>
Co-authored-by: Mathieu Ferment <mathieu.ferment@prestashop.com>
Copy link
Copy Markdown
Contributor

@jolelievre jolelievre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of this, although I have a few comments

$req: 1; $opt: 2; Number of arguments : 3
```

In our world, mixing the two ways
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a suggestion about redaction, it might be easier to understand if you first display the old way, and then the new way Starting the explanation with the new syntax first, which they may not know, is confusing as you don't have the context If you show the old context first at least people are in their comfort zone ready to accept the new one

In our world, mixing the two ways

```
private function formatProductsForAssociation(ProductForAssociation ...$productsForAssociation): array
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You only described this as method parameters but I believe it's usable for return as well no? The usage of this new feature will only have its real value if we use them both ways: input and output

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately variadic arguments are only used as input. But as output we could use a collection DTO that implements \iterable or \IteratorAggregate interface.

Copy link
Copy Markdown

@sowbiba sowbiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's kind of confusing since, according to your description, this operator can have 2 different meaning depending if it's typed or not, IF I understand well

function f($a, $b, ...$c)
means that you can pass any number of arguments to the function, when it's more than 2, they will be in the variable $c

f(1, 2, $d, $e, $f, $g) => count($c) in the function will give 4

But with function f($a, $b, Product ...$c)

you cannot do f(1, 2, $d, $e, $f, $g) even if $d, $e, $f, $g are Product
but you can do f(1, 2, [$d, $e, $f, $g])

I will look further to PHP documentation

@sowbiba
Copy link
Copy Markdown

sowbiba commented Jan 18, 2022

It's kind of confusing since, according to your description, this operator can have 2 different meaning depending if it's typed or not, IF I understand well

function f($a, $b, ...$c) means that you can pass any number of arguments to the function, when it's more than 2, they will be in the variable $c

f(1, 2, $d, $e, $f, $g) => count($c) in the function will give 4

But with function f($a, $b, Product ...$c)

you cannot do f(1, 2, $d, $e, $f, $g) even if $d, $e, $f, $g are Product but you can do f(1, 2, [$d, $e, $f, $g])

I will look further to PHP documentation

Or maybe in your 2nd example f(Product ...$products) is not equivalent to f(array $products) ?

Co-authored-by: Jonathan Lelievre <jo.lelievre@gmail.com>
@saulaski
Copy link
Copy Markdown

Even if it's PHP and I'm a front-end developer, I have to agree with this, no kind of types like in TS with Array<Products> in PHP?

Not yet unfortunately :/

@saulaski
Copy link
Copy Markdown

In case BC break is an issue, I suggest a simple trick:

public function setItems(array $items): self
{
    $this->items = [];

    return $this->addItem(...$items);
}

protected function addItem(Item ...$items): self
{
    foreach ($items as $item) {
        $this->items[] = $item;
    }

    return $this;
}

@saulaski
Copy link
Copy Markdown

It's kind of confusing since, according to your description, this operator can have 2 different meaning depending if it's typed or not, IF I understand well
function f($a, $b, ...$c) means that you can pass any number of arguments to the function, when it's more than 2, they will be in the variable $c
f(1, 2, $d, $e, $f, $g) => count($c) in the function will give 4
But with function f($a, $b, Product ...$c)
you cannot do f(1, 2, $d, $e, $f, $g) even if $d, $e, $f, $g are Product but you can do f(1, 2, [$d, $e, $f, $g])
I will look further to PHP documentation

Or maybe in your 2nd example f(Product ...$products) is not equivalent to f(array $products) ?

Variadic arguments are always received as an array in the function body. This array contains as many items as variadic arguments provided.
If you need to pass items in an array as variadic arguments, just use the spread operator : f(1, 2, ...$products)

@matks
Copy link
Copy Markdown
Contributor

matks commented Jan 24, 2022

Let's try to push this topic this week so what we can vote next week \o/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants