xp-framework/core

Make lang.Object -> lang.Value migration easier

thekid opened this issue · 2 comments

Scenario

A typical migration to XP9 will include replacing all extends \lang\Object with one of the two following:

  1. Nothing - in which case you're done.
  2. With implements \lang\Value - and then implementing hashCode(), compareTo() and toString().

The key point here is that implementing hashCode() and compareTo() can prove to be tedious, especially if I only want the added benefits of a nice toString() output.

Example

This is a diff in the xp-framework/webservices library where I really don't care much about comparing, but want the toString() method.

index 087a1a6..f187f51 100644
--- a/src/main/php/webservices/rest/RestResponse.class.php
+++ b/src/main/php/webservices/rest/RestResponse.class.php
@@ -9,7 +9,7 @@ use peer\http\HttpResponse;
  *
  * @test    xp://net.xp_framework.unittest.webservices.rest.RestResponseTest
  */
-class RestResponse {
+class RestResponse implements \lang\Value {
   protected $response= null;
   protected $reader= null;
   protected $input= null;
@@ -213,4 +213,23 @@ class RestResponse {
   public function toString() {
     return nameof($this).'<'.$this->response->message().'>@(->'.$this->response->toString().')';
   }
+
+  /**
+   * Creates a hashcode
+   *
+   * @return string
+   */
+  public function hashCode() {
+    return uniqid();
+  }
+
+  /**
+   * Comparison
+   *
+   * @param  var $value
+   * @return int
+   */
+  public function compareTo($value) {
+    return $value === $this ? 0 : 1;
+  }
 }

Suggestions

Either add a \lang\Stringable interface and support that in the relevant places or add a trait \lang\Comparison which implements hashCode() and compareTo() generically. Alternatively, simply check for toString() instead of testing with instanceof, kind of the magic PHP way...

Checking for toString()

Implemented as follows:

diff --git a/src/main/php/util/Objects.class.php b/src/main/php/util/Objects.class.php
index ed78350..eb41af1 100755
--- a/src/main/php/util/Objects.class.php
+++ b/src/main/php/util/Objects.class.php
@@ -71,7 +71,7 @@ abstract class Objects {
       return $val ? 'true' : 'false';
     } else if (is_int($val) || is_float($val)) {
       return (string)$val;
-    } else if (($val instanceof Value) && !isset($protect[$hash= (string)$val->hashCode()])) {
+    } else if (method_exists($val, 'toString') && !isset($protect[$hash= (string)$val->hashCode()])) {
       $protect[$hash]= true;
       $s= $val->toString();
       unset($protect[$hash]);

Performance 1.207 seconds for 1.000.000 calls for instanceof, 1.277 seconds for method_exists, so a very small impact for only that change; however, we can't rely on hashCode() being there anymore, and need to use spl_object_hash() or a version with SplObjectStorage(), bringing us to 1.307 seconds.

Here's a hack which would speed that up again. It's risky because it might trigger user-defined __isset, __set and __unset interceptors...

diff --git a/src/main/php/util/Objects.class.php b/src/main/php/util/Objects.class.php
index ed78350..0bb6ae4 100755
--- a/src/main/php/util/Objects.class.php
+++ b/src/main/php/util/Objects.class.php
@@ -71,10 +71,10 @@ abstract class Objects {
       return $val ? 'true' : 'false';
     } else if (is_int($val) || is_float($val)) {
       return (string)$val;
-    } else if (($val instanceof Value) && !isset($protect[$hash= (string)$val->hashCode()])) {
-      $protect[$hash]= true;
+    } else if (method_exists($val, 'toString') && !isset($val->__protect)) {
+      $val->__protect= true;
       $s= $val->toString();
-      unset($protect[$hash]);
+      unset($val->__protect);
       return $indent ? str_replace("\n", "\n".$indent, $s) : $s;
     } else if (is_array($val)) {
       if (empty($val)) return '[]';

An implementation with lang.Stringable would suffer from the same problem!

A Comparison trait

Declaration:

<?php namespace util;

use util\Objects;

trait Comparison {

  public function hashCode() {
    $s= self::class;
    foreach ((array)$this as $val) {
      $s.= '|'.Objects::hashOf($val);
    }
    return $s;
  }

  public function compareTo($value) {
    if ($value instanceof self) {
      foreach ((array)$this as $key => $val) {
        if (0 !== $r= Objects::compare($val, $value->{$key})) return $r;
      }
      return 0;
    }
    return 1;
  }
}

Note: The first "level" of hashOf() and compare() is inlined for performance reasons - 10ms for 1'000'000 calls

Usage:

<?php namespace com\example;

use util\Comparison;

class Person implements \lang\Value {
  use Comparison;

  public function toString(): string { return 'Person'; }
}

The compile time slowdown is almost not measurable and memory increase is actually better than if classes consist of duplicated code:

<?php namespace cmp;

use util\profiling\Timer;
use util\Comparison;
use lang\Runtime;

$timer= Timer::measure(function() {
  for ($i= 0; $i < 1000000; $i++) {
    $a= new class() { use Comparison; public $a; };
    $b= new class() { use Comparison; public $b; };
    $c= new class() { use Comparison; public $c; };
  }
});

printf("%.3f seconds, %.3f KB", $timer->elapsedTime(), Runtime::peakMemoryUsage() / 1024);

As seen above: 0.283 seconds, 998.125 KB
With use inlined: 0.281 seconds, 1001.430 KB