Collection's append() and prepend() behave differently when single argument provided

Kirby 3 Solved ✅
8

I actually went and compared Kirby2 and Kirby3 reference and code for append() and prepend()

In K2 it seems both append and prepend required both key and page to be provided. And prepend used union operator to add the key->object, as an array, to itself

  /**
   * Appends an element to the data array
   *
   * @param string $key
   * @param mixed $object
   * @return Collection
   */
  public function append($key, $object) {
    $this->data = $this->data + array($key => $object);
    return $this;
  }

  /**
   * Prepends an element to the data array
   *
   * @param string $key
   * @param mixed $object
   * @return Collection
   */
  public function prepend($key, $object) {
    $this->data = array($key => $object) + $this->data;
    return $this;
  }

… while pages->add() was the only method that did not require a key to be specified, probably because you can pass either a page or a collection of pages as argument, so if not present the method obtains the key, just like k3 collection->append() (but NOT k3 collection->prepend()):

  /**
   * Adds a single page object to the
   * collection by id or the entire object
   *
   * @param mixed $page
   */
  public function add($page) {

    if(is_a($page, 'Collection')) {
      foreach($page as $object) $this->add($object);
    } else if(is_string($page) and $object = page($page)) {
      $this->data[$object->id()] = $object;
    } else if(is_a($page, 'Page')) {
      $this->data[$page->id()] = $page;
    }

    return $this;

  }

In the reference, the need to provide two arguments to append and prepend was much clear in K2 imo (see links).

In k3 append() the key is ‘optional’ in the sense that if it is not provided, and the element appended is an object the method will find the key for you.

public function append(...$args)
{
    if (count($args) === 1) {
        if (is_object($args[0]) === true) {
            $this->data[$args[0]->id()] = $args[0];
        } else {
            $this->data[] = $args[0];
        }
    } elseif (count($args) === 2) {
        $this->set($args[0], $args[1]);
    }
    return $this;
}

While in K3 prepend(), as said, not providing a key results in the element being added without key, as the method uses array_unshift().

public function prepend(...$args)
{
    if (count($args) === 1) {
        array_unshift($this->data, $args[0]);
    } elseif (count($args) === 2) {
        $data = $this->data;
        $this->data = [];
        $this->set($args[0], $args[1]);
        $this->data += $data;
    }
    return $this;
}

This could be considered ‘optional’ too, I guess, but I do not think it provides the expected results, and the lack of key will probably make some possibly chained methods fail or provide an unexpected result. perhaps?

Plus this difference becomes obvious only by checking the code.

I am quite a newbie in php, but wouldn’t it be better that prepend does exactly as append, checking if element is an object, then finding its key, and calling itself with two arguments? reverting to unshift only when the element is NOT and object?

public function prepend(...$args)
{
    if (count($args) === 1) {
        if (is_object($args[0]) === true) {
            $this->prepend($args[0]->id(), $args[0]);
        } else {
            array_unshift($this->data, $args[0]);
        }
    } elseif (count($args) === 2) {
        $data = $this->data;
        $this->data = [];
        $this->set($args[0], $args[1]);
        $this->data += $data;
    }
    return $this;
}