# What is the prevailing sentiment about extract() and compact() ?

- Thread: https://externals.io/message/121844
- Original archive: https://news-web.php.net/php.internals/121844
- Started by Juliette Reinders Folmer on 2023-11-29T01:49:19+00:00
- Messages: 10
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #121844 — Juliette Reinders Folmer — 2023-11-29T01:49:19+00:00

URL: https://externals.io/message/121844#121844

L.S.,

What with all the drives towards cleaner code, how do people feel 
nowadays about `extract()` and `compact()` still being supported ?

Both have alternatives. The alternatives may be a little more cumbersome 
to type, but also make the code more descriptive, lessens the risk of 
variable name collisions (though this can be handled via the $flags in 
extract), prevents surprises when a non-associative key would be 
included in an array and lessens security risks when used on untrusted data

function foo() {
     $array = [
         'color' => 'blue',
         'size'  => 'medium',
     ];

     // Using extract.
     extract($array);
     var_dump($color);

     // Not using extract.
     var_dump($array['color']);

     $color = $array['color'];
     var_dump($color);
}

function bar( $color, $size ) {
     // Using compact.
     $array = compact('color', 'size');
     var_dump($array);

     // Not using compact.
     $array = [
         'color' => $color,
         'size'  => $size,
     ];
     var_dump($array);

     $array = [];
     foreach (['color', 'size'] as $name) {
         if (isset($$name)) {
             $array[$name] =  $$name;
         }
     }
     var_dump($array);
}

https://3v4l.org/JeHnY


I can imagine these could be candidates for deprecation ? Or limited 
deprecation - only when used in the global namespace ?

For now, I'm just wondering how people feel about these functions.

Smile,
Juliette

---

## #121845 — Larry Garfield — 2023-11-29T02:58:54+00:00

URL: https://externals.io/message/121844#121845 · In reply to: #121844

> L.S.,
>
> What with all the drives towards cleaner code, how do people feel 
> nowadays about `extract()` and `compact()` still being supported ?
>
> Both have alternatives. The alternatives may be a little more cumbersome 
> to type, but also make the code more descriptive, lessens the risk of 
> variable name collisions (though this can be handled via the $flags in 
> extract), prevents surprises when a non-associative key would be 
> included in an array and lessens security risks when used on untrusted data

*snip*

> I can imagine these could be candidates for deprecation ? Or limited 
> deprecation - only when used in the global namespace ?
>
> For now, I'm just wondering how people feel about these functions.
>
> Smile,
> Juliette

 `extract()` has very limited use in some kinds of template engine, which use PHP require() as a template mechanism.  I don't think `compact()` has any uses.

I very recently was just reminded that these even exist, as i had to tell one of my developers to not use them.  I think it was `compact()` he was trying to use.  I vetoed it.

I would not mind if they were removed, but I don't know how large the BC impact would be.  They'd probably need a long deprecation period, just to be safe.

--Larry Garfield

---

## #121850 — Stephen Reay — 2023-11-29T07:19:24+00:00

URL: https://externals.io/message/121844#121850 · In reply to: #121845

> 
> 
> 
> > […]
> 
> *snip*
> 
> > […]
> 
> `extract()` has very limited use in some kinds of template engine, which use PHP require() as a template mechanism.  I don't think `compact()` has any uses.
> 
> I very recently was just reminded that these even exist, as i had to tell one of my developers to not use them.  I think it was `compact()` he was trying to use.  I vetoed it.
> 
> I would not mind if they were removed, but I don't know how large the BC impact would be.  They'd probably need a long deprecation period, just to be safe.
> 
> --Larry Garfield
> 
> -- 
> 
> To unsubscribe, visit: https://www.php.net/unsub.php
> 

Hi,

While I think I understand the goal behind this, I think you're missing some factors here.

Regarding use-cases for compact: the most common one I can think of from my work, is for passing multiple local variables as context to a logging function, but I'd be surprised if its not also used to build faux hash structures too.

If your goal is to achieve an associative array (i.e a poor mans hash) of known variable names, using compact in php8+ has *less* risk of uncaught/unexpected errors than building it manually. Passing an undefined name (i.e. due a typo, or it just not being defined) produces a warning regardless of whether you build the array manually or pass the name(s) to `compact()`. Providing an array key name that doesn't match the variable name (e.g. due to a typo, or a variable being renamed) will produce no error when building the array manually, but will produce a warning with `compact()`. 

IDEs (e.g. PHPStorm/IDEA+PHP plugin) can already understand that the names passed to compact are a variable name, and make changes when a variable is renamed via the IDE. They simply cannot do the same for plain array keys.

Due to how variable scope works, the only way to re-implement `compact()` with the same key-typo-catching behaviour as a function in userland would be something that requires the user to pass the result of `get_defined_vars()` to every call.

So no, I don't think `compact()` should be deprecated, what I think *should* happen, is to promote the current warning on undefined variables, to an error, as per https://wiki.php.net/rfc/undefined_variable_error_promotion. Whether this is a foregone conclusion or not, I don't know because that RFC doesn't mention `compact()` specifically.


 `extract()`, as Larry points out has historically been used by 'pure php' style template systems, in a manner that's generally "safe". Personally I'm less inclined to use this behaviour now (i.e. I'd prefer to access named & typed properties from a template than arbitrary local variable names) but I don't think that's enough of a case to remove it, because just like with compact, by nature of how variable scope works, it's very difficult/impossible to re-implement this in userland, in a way that's reusable and doesn't involve using worse constructs (e.g. eval'ing the result of a function)

I think there's possibly an argument to be made for improvements, such as changing the default mode of extract to something besides EXTR_OVERWRITE, or to have checks in place preventing the overwrite of superglobals.


Cheers


Stephen

---

## #121852 — Robert Landers — 2023-11-29T08:20:04+00:00

URL: https://externals.io/message/121844#121852 · In reply to: #121850

>
>
>
> > […]
>
> Hi,
>
> While I think I understand the goal behind this, I think you're missing some factors here.
>
> Regarding use-cases for compact: the most common one I can think of from my work, is for passing multiple local variables as context to a logging function, but I'd be surprised if its not also used to build faux hash structures too.
>
> If your goal is to achieve an associative array (i.e a poor mans hash) of known variable names, using compact in php8+ has *less* risk of uncaught/unexpected errors than building it manually. Passing an undefined name (i.e. due a typo, or it just not being defined) produces a warning regardless of whether you build the array manually or pass the name(s) to `compact()`. Providing an array key name that doesn't match the variable name (e.g. due to a typo, or a variable being renamed) will produce no error when building the array manually, but will produce a warning with `compact()`.
>
> IDEs (e.g. PHPStorm/IDEA+PHP plugin) can already understand that the names passed to compact are a variable name, and make changes when a variable is renamed via the IDE. They simply cannot do the same for plain array keys.
>
> Due to how variable scope works, the only way to re-implement `compact()` with the same key-typo-catching behaviour as a function in userland would be something that requires the user to pass the result of `get_defined_vars()` to every call.
>
> So no, I don't think `compact()` should be deprecated, what I think *should* happen, is to promote the current warning on undefined variables, to an error, as per https://wiki.php.net/rfc/undefined_variable_error_promotion. Whether this is a foregone conclusion or not, I don't know because that RFC doesn't mention `compact()` specifically.
>
>
> `extract()`, as Larry points out has historically been used by 'pure php' style template systems, in a manner that's generally "safe". Personally I'm less inclined to use this behaviour now (i.e. I'd prefer to access named & typed properties from a template than arbitrary local variable names) but I don't think that's enough of a case to remove it, because just like with compact, by nature of how variable scope works, it's very difficult/impossible to re-implement this in userland, in a way that's reusable and doesn't involve using worse constructs (e.g. eval'ing the result of a function)
>
> I think there's possibly an argument to be made for improvements, such as changing the default mode of extract to something besides EXTR_OVERWRITE, or to have checks in place preventing the overwrite of superglobals.
>
>
> Cheers
>
>
> Stephen

FWIW, I use compact all the time, usually like this:

try {
  // do stuff
} catch(Throwable $exception) {
  $this->logger->error("failed to do stuff", compact('exception'));
  throw $exception;
}

But thanks for the reminder to finish the nameof RFC, I was waiting
until after 8.3 to avoid the "trying to rush it to get into 8.3"
shenanigans that happened to another RFC around the same time. If
nameof passes, then it could make this more obvious when refactoring:

try {
  // do stuff
} catch(Throwable $exception) {
  $this->logger->error("failed to do stuff", compact(nameof($exception)));
}

Robert Landers
Software Engineer
Utrecht NL

---

## #121853 — Ayesh Karunaratne — 2023-11-29T08:36:07+00:00

URL: https://externals.io/message/121844#121853 · In reply to: #121852

>
> try {
>   // do stuff
> } catch(Throwable $exception) {
>   $this->logger->error("failed to do stuff", compact('exception'));
>   throw $exception;
> }
>

I wonder why not just create an array with the key...

```php
try {
   // do stuff
} catch(Throwable $exception) {
   $this->logger->error("failed to do stuff", ['exception' => $exception]));
   throw $exception;
}
```

It's a few more characters of course, but I would pick the readability
and simplicity over the potential typos (which the IDE should point
out) and a few key presses it saves.I

---

## #121854 — Lynn — 2023-11-29T08:55:59+00:00

URL: https://externals.io/message/121844#121854 · In reply to: #121852

On Wed, Nov 29, 2023 at 9:20 AM Robert Landers <landers.robert@gmail.com>
wrote:

> On Wed, Nov 29, 2023 at 8:19 AM Stephen Reay <php-lists@koalephant.com>
> wrote:
> > […]
> wrote:
> > […]
> cumbersome
> > […]
> untrusted data
> > […]
> use PHP require() as a template mechanism.  I don't think `compact()` has any
> uses.
> > […]
> tell one of my developers to not use them.  I think it was `compact()` he was
> trying to use.  I vetoed it.
> > […]
> BC impact would be.  They'd probably need a long deprecation period, just
> to be safe.
> > […]
> some factors here.
> > […]
> my work, is for passing multiple local variables as context to a logging
> function, but I'd be surprised if its not also used to build faux hash
> structures too.
> > […]
> of known variable names, using compact in php8+ has *less* risk of
> uncaught/unexpected errors than building it manually. Passing an undefined
> name (i.e. due a typo, or it just not being defined) produces a warning
> regardless of whether you build the array manually or pass the name(s) to
> `compact()`. Providing an array key name that doesn't match the variable name
> (e.g. due to a typo, or a variable being renamed) will produce no error
> when building the array manually, but will produce a warning with `compact()`.
> > […]
> names passed to compact are a variable name, and make changes when a
> variable is renamed via the IDE. They simply cannot do the same for plain
> array keys.
> > […]
> with the same key-typo-catching behaviour as a function in userland would
> be something that requires the user to pass the result of
> `get_defined_vars()` to every call.
> > […]
> *should* happen, is to promote the current warning on undefined variables,
> to an error, as per
> https://wiki.php.net/rfc/undefined_variable_error_promotion. Whether this
> is a foregone conclusion or not, I don't know because that RFC doesn't
> mention `compact()` specifically.
> > […]
> style template systems, in a manner that's generally "safe". Personally I'm
> less inclined to use this behaviour now (i.e. I'd prefer to access named &
> typed properties from a template than arbitrary local variable names) but I
> don't think that's enough of a case to remove it, because just like with
> compact, by nature of how variable scope works, it's very
> difficult/impossible to re-implement this in userland, in a way that's
> reusable and doesn't involve using worse constructs (e.g. eval'ing the
> result of a function)
> > […]
> as changing the default mode of extract to something besides
> EXTR_OVERWRITE, or to have checks in place preventing the overwrite of
> superglobals.
> > […]
>
> FWIW, I use compact all the time, usually like this:
>
> try {
>   // do stuff
> } catch(Throwable $exception) {
>   $this->logger->error("failed to do stuff", compact('exception'));
>   throw $exception;
> }
>
> But thanks for the reminder to finish the nameof RFC, I was waiting
> until after 8.3 to avoid the "trying to rush it to get into 8.3"
> shenanigans that happened to another RFC around the same time. If
> nameof passes, then it could make this more obvious when refactoring:
>
> try {
>   // do stuff
> } catch(Throwable $exception) {
>   $this->logger->error("failed to do stuff", compact(nameof($exception)));
> }
>
> Robert Landers
> Software Engineer
> Utrecht NL
>
> --
> 
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>
My main concern with compact and extract is the counterintuitive usage of
variable names, which is also why I've personally broken production after
changing a variable name. If there is another way of using compact in a way
where it's used without string and with dollar sign, I won't have a problem
with it. I still find `compact(nameof($exception))` a little confusing
personally though. I have the feeling that the wish is to have a nice
syntactic sugar for compact, not sure about extract though. Extract for me
is somewhat of a security concern as it will make it easy to implicitly
overwrite variables from a local scope if the array being used is filled
elsewhere. Someone adding an array key in another layer of code in an
application can cause different behavior on a totally unrelated place when
there's a variable collision, and there's no way to detect this when adding
(or removing) a key from the array.

I'm all for getting rid of extract. For compact I'd like to explore
alternative solutions before outright deprecating. I'm still in favor of
seeing it gone in its current form though.

---

## #121855 — Robert Landers — 2023-11-29T09:02:20+00:00

URL: https://externals.io/message/121844#121855 · In reply to: #121853

Robert Landers
Software Engineer
Utrecht NL


>
> > […]
>
> I wonder why not just create an array with the key...
>
> ```php
> try {
>    // do stuff
> } catch(Throwable $exception) {
>    $this->logger->error("failed to do stuff", ['exception' => $exception]));
>    throw $exception;
> }
> ```
>
> It's a few more characters of course, but I would pick the readability
> and simplicity over the potential typos (which the IDE should point
> out) and a few key presses it saves.I

Hey Ayesh,

Yeah, this is very much a personal preference kind of thing. I choose
compact not because it is shorter to type, but because typos are a
thing if you've been around PHP for awhile... who hasn't opened a
function written 15 years ago with a misspelled variable that you want
to fix but it has been that way for 15 years, so you don't fix it.

I've also used compact inside array_maps like:

$arr = array_map(function($a) {
  $name = $a['first'] . " " . $a['last'];
  // more mappings
  return compact('name', ...);
}, $arr);

just because it is easier to type/read than a bunch of array accesses.
Possibly faster as well, but if it is, it's by nano-seconds.

Robert Landers
Software Engineer
Utrecht NL

---

## #121856 — Robert Landers — 2023-11-29T09:08:18+00:00

URL: https://externals.io/message/121844#121856 · In reply to: #121854

>
>
>
> 
> > […]
>
> My main concern with compact and extract is the counterintuitive usage of variable names, which is also why I've personally broken production after changing a variable name. If there is another way of using compact in a way where it's used without string and with dollar sign, I won't have a problem with it. I still find `compact(nameof($exception))` a little confusing personally though. I have the feeling that the wish is to have a nice syntactic sugar for compact, not sure about extract though. Extract for me is somewhat of a security concern as it will make it easy to implicitly overwrite variables from a local scope if the array being used is filled elsewhere. Someone adding an array key in another layer of code in an application can cause different behavior on a totally unrelated place when there's a variable collision, and there's no way to detect this when adding (or removing) a key from the array.
>
> I'm all for getting rid of extract. For compact I'd like to explore alternative solutions before outright deprecating. I'm still in favor of seeing it gone in its current form though.

It would be neat to combine something like nameof into something more intuitive:

$name = "Robert";
$title = "Software Engineer";

$arr = [ $name => ..., $title => ... ];

// $arr == ['name' => 'Robert', 'title' => 'Software Engineer']

would basically mean take the name of the variable as the key and the
value as the value of the array. If we took the same semantics as
nameof, then even this would work:

$arr = [ $this->name => ..., $this->title => ... ];

// $arr == ['name' => 'Robert', 'title' => 'Software Engineer']

I'm sure there is probably a prettier way to express it, but I don't
dislike it. Then we just deprecate compact. There's already
destructuring to handle extract's case:

[ 'name' => $name, 'title' => $title ] = $arr;

Robert Landers
Software Engineer
Utrecht NL

---

## #121863 — David Gebler — 2023-11-29T20:17:06+00:00

URL: https://externals.io/message/121844#121863 · In reply to: #121850

On Wed, Nov 29, 2023 at 7:19 AM Stephen Reay <php-lists@koalephant.com>
wrote:

> So no, I don't think `compact()` should be deprecated, what I think *should*
> happen, is to promote the current warning on undefined variables, to an
> error, as per https://wiki.php.net/rfc/undefined_variable_error_promotion.
> Whether this is a foregone conclusion or not, I don't know because that RFC
> doesn't mention `compact()` specifically.
>
>
I remember fixing a bug in compact where passing parameters which were not
array or string were silently accepted - something I discovered when I was
tracking down a bug in my code once and it turned out I'd accidentally done
compact($x) instead of compact('x') - and that now gives a warning,
consistent with the warning when variable names given are undefined.

I don't want to see either compact or extract deprecated because I do use
both (as do countless open source systems) and I don't think there is a
reasonable justification for deprecating them. The behaviour of these
functions is understood by static analysis tools, the alternatives are more
verbose and more prone to developer error / mistakes from typos. I do agree
with upping the error level in `compact()` from `E_WARNING` to `E_ERROR` and
upping the warning on bad parameter types to a TypeError (which is what I'd
originally pushed for), but I think these things should be a change to
compact specifically, separate from any other RFC.

-Dave

---

## #121896 — Marco Pivetta — 2023-12-01T12:33:18+00:00

URL: https://externals.io/message/121844#121896 · In reply to: #121844

Hey Juliette,

On Wed, 29 Nov 2023 at 03:00, Juliette Reinders Folmer <
php-internals_nospam@adviesenzo.nl> wrote:

>
> For now, I'm just wondering how people feel about these functions.
>

From my PoV: burn them with fire.
The translation of `compact()` is **trivial** with a CS tool (like the one
you maintain), and the translation of `extract($vars)` is `foreach ($vars
as $k => $v) { $$k = $v; }`, and that's all there is.

Instead, both `compact()` and `extract()` come at additional cognitive load
when reading, scanning, analyzing, debugging and generally understanding
code: a flamethrower helps.

Meanwhile, I'm also wondering if the removal of such functions would help
more in future AOT and JIT improvements: I suppose code using `extract()`
and `compact()` would immediately defeat any JIT, or at least lead to added
code to handle them correctly?

There's a section describing this in HHVM, which is the kind of
problem-space I was thinking of:
https://hhvm.com/blog/713/hhvm-optimization-tips


Marco Pivetta

https://mastodon.social/@ocramius

https://ocramius.github.io/