Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HttpFoundation] The ability to add a new elements to an array without key #30336

Open
PlanetGamingGG opened this Issue Feb 21, 2019 · 5 comments

Comments

Projects
None yet
5 participants
@PlanetGamingGG
Copy link

PlanetGamingGG commented Feb 21, 2019

Description
Adds a new array element without setting the key. Equivalent to $array[] = 'Stuff I wanted to add';.

Example

<?php

// Init
$session->set('array', []);

# How I currently add stuff:
$array = $session->get('array');
$array[] = 'Something';
$session->set('array', $array);

# What I wanted it to be:
$session->add('array', 'Something');
# OR
$session->set('array[]', 'Something');
@linaori

This comment has been minimized.

Copy link
Contributor

linaori commented Feb 22, 2019

While certainly possible (see property accessor), I'm unsure of the DX this brings. Looking at the first example, I know the type and how to use it. In the second, it's unclear. In the first example, there's no guarantee the key is set or that this value is an array though, so you rely on php's error-handling/behavior for this. In the second case, Symfony would have to reproduce the php behavior and it sounds like something that's bound to be fragile and performance intensive compared to the first example.

I've personally tried to avoid using the session as shown above. I try to solve it by storing (immutable) data structures (objects) in the session whenever something is needed. If the object is mutable, you can mutate it without setting it back into the session due to object references.

The above example could be done as:

$foo = new Foo([]);
$session->set('foo', $foo);

// next request
if ($session->has('foo')) {
    $foo = $session->get('foo');
    if ($foo instanceof Foo) {
        $foo->add('bar');
        // or perhaps
        $foo->someData[] = 'bar';
    }
}
@PlanetGamingGG

This comment has been minimized.

Copy link
Author

PlanetGamingGG commented Feb 22, 2019

Good point @linaori ,
I wanted to make the code a bit more elegant and shorter.
About the guarantee that it will return an array and in a scenario where you believe this can either be an array or a string, I can propose this:

<?php

/**
 * Example 1
 */
# Add Something!
$data = $session->get('anArray:array');
$data[] = 'Something';
$session->set('anArray', $data);

/**
 * Example 2
 */
// The data was set somewhere.. It might be
// an array, it might be a string or a bird?

# Add Something!
$session->add('anArray', 'Something'); 
// If "anArray" IS NOT an Array, it will throw an exception.

Example 1 and 2 assumes you DID NOT KNOW the data type.

In Example 1, the colon after the key name is the type you wanted. In the example it was set to array. It can be any valid PHP data types such as bool, string, int, etc. so this way, we can be sure that we get the correct data type. Otherwise an Exception shall be thrown.

Example 2 is my original proposal which is the add method. The method shall only works on arrays and will throw an Exception if it is not an array.

@PlanetGamingGG

This comment has been minimized.

Copy link
Author

PlanetGamingGG commented Feb 22, 2019

Regarding my comments above, I just realized that catching Exceptions would be the new problem....
Either $session->get('anArray:array'); would return an empty array or whatever it is casted to an array.

Assuming anArray = [].
$session->get('anArray:array'); = []
$session->get('anArray:string'); = '' Empty string
$session->get('anArray:bool'); = false
And yeah this requires more checks.

Also for the Example 2, same issues, Exceptions can be the new problem.
How about $session->add('anArray', 'Something'); shall return boolean.
Example:

<?php

if($session->add('anArray', 'Something')){
	// Successfully added "Something"
}else{
	// Failed to add "Something"...
	// Probably because "anArray" is not an array.
}
@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Feb 24, 2019

Keys are opaque strings - ie they're not parsed. I think that's an important design choice we'd better keep as is: less code to maintain, less idiomatic to teach, better reuse of basic and common language features everyone should better learn that this kind of new syntax here.

Not convinced on my side.

@xabbuh

This comment has been minimized.

Copy link
Member

xabbuh commented Feb 26, 2019

I tend to agree with @nicolas-grekas. However, if you really need this, writing a layer on-top of the Session class that leverages the PropertyAccess component should be doable in userland code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.