Arrays.pop() fn: pop element from the array by key and return its value

[foxycode]

I hope this could be good contribution to Array utils. I'm using this all the time.

[JanTvrdik]

The name is terrible because it does something very different from array_pop.

[JanTvrdik]

The implementation could be simplified

if (array_key_exists($key, $arr)) { // or maybe isset?
    $value = $arr[$key];
    unset($arr[$key]);
    return $value;

} elseif (func_num_args() < 3) {
    throw new Nette\InvalidArgumentException("Missing item '$key'.");

} else {
    return $default;
}

[foxycode]

I actually tried to stick with conventions and used get function as template.

[JanTvrdik]

get method has its reasons why it must look the way it does, but those reasons are not relevant to this function

[foxycode]

Why not? I see it working same way. If first argument is not array, you get provided default value like in get function. It could be odd to trigger error instead. So I decided is_array actually is important.

[foxycode]

You are right, but I don't know better name. Pop function is used like this in Python. Maybe we can make it compatible with array_pop by making $key argument optional?

[foxycode]

@JanTvrdik Upravil jem implementaci dle připomínek, snad to takto bude vyhovovat.

[dg]

What about pick?

[foxycode]

@dg pop is convention for removing element from array (list, dict, etc.), pick does not sound like for that purpose to me. But me personally does not insist on this specific name.

[dg]

here is missing free line

[foxycode]

Updated

[vojtech-dobes]

Arrays::pick sounds very appropriate to me and the pop functionality could be removed from it (to keep users using normal array_pop when "poping" without key is needed).

[dg]

Push & Pop are instructions older than me ;) used to work with the top of stack. How pop is enhanced in Python is interesting, but nowhere else it is used this way.

[foxycode]

@dg Should I remove unneeded is_array from get function too?

[dg]

No, it is needed.

[foxycode]

@dg I don't understand why if $arr type is forced in function arguments. I expect there will be error if you call get with non-array argument?

FYI: pop works like this in ruby too and even C++ map have it this way.

[vojtech-dobes]

$arr gets rewritten because of recursive going throught the array (if I get it correctly).

[foxycode]

@vojtech-dobes Now I see it, thanks for explanation.

@dg I rebased to one commit. Should I update method name to pick and you merge it or can I wait for other comments?

[dg]

When you will rename it to pick probably pop functionality can be removed, as mentioned #54 (comment).

[xificurk]

I don't like pick, because it's not really obvious that it actually removes the item form the array. I'm biased towards pop for the same reasons as @foxycode (usage in other languages like Python).

[dg]

@xificurk so what do you think that Arrays:pick($arr, $key) does? And is pop used this way in other language than Python?

[xificurk]

@dg Well, the term "pick" is used in PHP documentation e.g. here:

array_rand — Pick one or more random entries out of an array

Regarding other languages see #54 (comment). Is there any other language that uses pick get and remove item from an array?

[dg]

That doesn't seems to me…

• http://ruby-doc.org//core-2.2.0/Array.html#method-i-pop
• http://www.cplusplus.com/reference/stack/stack/pop/
• strpos('array_rand', 'pick') === FALSE

[foxycode]

@dg @xificurk Looks like I was wrong.

Ruby have parameter in array pop function, but for other reason: http://www.rubydoc.info/stdlib/core/1.9.3/Array:pop

C++ map have find and erase methods, pop by key can't be done in one step.

Python: https://docs.python.org/2/library/stdtypes.html#dict.pop

Main difference between PHP a Python in this scenario is, Python have separate dict and list types. PHP combines these two types into one.

If function name should be other than pop, I vote for remove reather than pick, but I don't see problem with pop which is now compatible with array_pop.

[foxycode]

One more thing gentlemans. Main reason I wanted to have pop in Arrays was include it in ArrayHash too. The constantly repeating scenario:

public function formSuccess(Form $form, ArrayHash $values)
{
    $id = $values->id;
    unset($values->id);
    $this->someRepository->update($id, $values);
}

could be reduced to

public function formSuccess(Form $form, ArrayHash $values)
{
    $this->someRepository->update($values->pop('id'), $values);
}

What do you think about it?

[dg]

ArrayHash contains no method and pop() doesn't seem to be so important to "pollute" pure class…

[dg]

ad remove(): it is ok, but expected return value is bool. And remove($key, $default) is strange.

[foxycode]

@dg Yes, ArrayHash contains no method, so what about adding some more from Arrays, not just one? :-)

Ad remove: what about cut?

[JanTvrdik]

it is ok, but expected return value is bool

Not really, see

• http://docs.oracle.com/javase/7/docs/api/java/util/Map.html#remove%28java.lang.Object%29
• http://ruby-doc.org//core-2.1.1/Hash.html#method-i-delete
• https://api.dartlang.org/apidocs/channels/be/dartdoc-viewer/dart:core.Map#id_remove
• http://www.scala-lang.org/api/current/index.html#scala.collection.mutable.HashMap@remove%28key:A%29:Option%5BB%5D

Only C# does return bool and C++ 0/1.

[foxycode]

@dg I'd really like to have it in Nette 2.3, so, what should I change?

[JanTvrdik]

@foxycode Even if we add Arrays::remove() it will not work for ArrayHash. I'm not sure there is anything Nette can do for you in this regard.

[foxycode]

@JanTvrdik I'm ok with Arrays::remove() too :-) Extend ArrayHash was just a suggestion.

[MartinMystikJonas]

Just idea: what about takeOut?

[dg]

As I said, remove($key) is for me ok, but remove($key, $default) is weird. Also remove with optional $key.

[foxycode]

@dg Should I therefore rename to pick and remove optinal $key? I settle with that.

Or what you see wrong on pop if it is backward compatible with array_pop like this?

[dg]

I guess I has already lost in this discussion :-)

[dg]

@foxycode if you want something like this:

    public static function remove(array & $arr, $key)
    {
        $value = isset($arr[$key]) ? $arr[$key] : NULL;
        unset($arr[$key]);
        return $value;
    }

it is ok, do it. Or do you want something more sofisticated? Pop that is backward compatible with array_pop? Why? Or do you want to use name pick? You said that you prefer remove? Why now pick?

[foxycode]

@dg I originally chosed pop becaue it removes element from array and it is used that way in python. I added array_pop backward compatibility persuant comment from @JanTvrdik

I will be satisfied with pick too, but it doesn't sounds to me like actually removing element from array. I really don't like delete either. What about cut or pull?

I can definitely remove array_pop compatibility, but I'd like to keep default value, is it possible? I rebased commit to name pick and removed array_pop part. If it is ok for you this way, you can merge it :-)

[dg]

Aha, so $default is for you important, yes?

[dg]

http://martinfowler.com/bliki/TwoHardThings.html

[foxycode]

@dg $default not so much, but throwing exception when $key does not exists is.

[foxycode]

@dg Could I do something to push this further?