fix todo comments

DataStorage/Database/DataMapperAbstract.php

@@ -410,8 +410,6 @@ class DataMapperAbstract implements DataMapperInterface
 
         // clear parent and objects
         if (static::class === self::$parentMapper) {
-            //self::$initObjects = []; // todo: now all objects are cached for the whole request
-            //self::$initArrays = []; // todo: now all objects are cached for the whole request
             self::$parentMapper = null;
         }
     }
@@ -461,7 +459,15 @@ class DataMapperAbstract implements DataMapperInterface
 
         $refClass = new \ReflectionClass($obj);
 
-        // todo: remove force and instead check if object is autoincrement. if not autoincrement then create even if id is set!!!
+        /**
+         * @todo Orange-Management/phpOMS#234
+         *  Add forced IDs to the mappers
+         *  Currently it is required to manually create a model with forced ids.
+         *  This should be defined in the mappers in the ID column definition.
+         *  ```
+         *      'force_id' => true,
+         *  ```
+         */
         if (!empty($id = self::getObjectId($obj, $refClass)) && !$force) {
             $objId = $id;
         } else {
@@ -561,12 +567,8 @@ class DataMapperAbstract implements DataMapperInterface
                 $query->insert($column['name'])->value($value);
             } elseif ($column['name'] !== static::$primaryField || !empty($property->getValue($obj))) {
                 $tValue = $property->getValue($obj);
-                if (\stripos($column['internal'], '/') !== false) {
+                if (\stripos($column['internal'], '/') === 0) {
-                    $path = \explode('/', $column['internal']);
+                    $tValue = ArrayUtils::getArray($column['internal'], $tValue, '/');
-
-                    \array_shift($path);
-                    $path   = \implode('/', $path);
-                    $tValue = ArrayUtils::getArray($path, $tValue, '/');
                 }
 
                 $value = self::parseValue($column['type'], $tValue);
@@ -633,14 +635,11 @@ class DataMapperAbstract implements DataMapperInterface
             }
 
             $path = $column['internal'];
-            if (\stripos($column['internal'], '/') !== false) {
+            if (\stripos($column['internal'], '/') === 0) {
-                $path = \explode('/', $column['internal']);
+                $path = \ltrim($column['internal'], '/');
-
-                \array_shift($path); // todo: why am I doing this?
-                $path = \implode('/', $path);
             }
 
-            $property = ArrayUtils::getArray($path, $obj, '/');
+            $property = ArrayUtils::getArray($column['internal'], $obj, '/');
 
             if (isset(static::$ownsOne[$path])) {
                 $id    = self::createOwnsOneArray($column['internal'], $property);
@@ -1194,7 +1193,6 @@ class DataMapperAbstract implements DataMapperInterface
             $objsIds[$propertyName] = [];
 
             foreach ($values as $key => &$value) {
-                // todo: carefull what if a value is an object or another array?
                 if (!\is_array($value)) {
                     // Is scalar => already in database
                     $objsIds[$propertyName][$key] = $value;
@@ -1309,8 +1307,6 @@ class DataMapperAbstract implements DataMapperInterface
         /** @var string $mapper */
         $mapper = static::$ownsOne[$propertyName]['mapper'];
 
-        // todo: delete owned one object is not recommended since it can be owned by by something else? or does owns one mean that nothing else can have a relation to this one?
-
         return $mapper::update($obj, $relations, $depth);
     }
 
@@ -1429,23 +1425,30 @@ class DataMapperAbstract implements DataMapperInterface
                 $id    = self::updateOwnsOne($propertyName, $property->getValue($obj), $relations, $depth);
                 $value = self::parseValue($column['type'], $id);
 
-                // todo: should not be done if the id didn't change. but for now don't know if id changed
+                /**
+                 * @todo Orange-Management/phpOMS#232
+                 *  If a model gets updated all it's relations are also updated.
+                 *  This should be prevented if the relations didn't change.
+                 *  No solution yet.
+                 */
                 $query->set([static::$table . '.' . $column['name'] => $value]);
             } elseif (isset(static::$belongsTo[$propertyName])) {
                 $id    = self::updateBelongsTo($propertyName, $property->getValue($obj), $relations, $depth);
                 $value = self::parseValue($column['type'], $id);
 
-                // todo: should not be done if the id didn't change. but for now don't know if id changed
+                /**
+                 * @todo Orange-Management/phpOMS#232
+                 *  If a model gets updated all it's relations are also updated.
+                 *  This should be prevented if the relations didn't change.
+                 *  No solution yet.
+                 */
                 $query->set([static::$table . '.' . $column['name'] => $value]);
             } elseif ($column['name'] !== static::$primaryField) {
                 $tValue = $property->getValue($obj);
-                if (\stripos($column['internal'], '/') !== false) {
+                if (\stripos($column['internal'], '/') === 0) {
-                    $path = \explode('/', $column['internal']);
+                    $tValue = ArrayUtils::getArray($column['internal'], $tValue, '/');
-
-                    \array_shift($path);
-                    $path   = \implode('/', $path);
-                    $tValue = ArrayUtils::getArray($path, $tValue, '/');
                 }
+
                 $value = self::parseValue($column['type'], $tValue);
 
                 $query->set([static::$table . '.' . $column['name'] => $value]);
@@ -1492,13 +1495,10 @@ class DataMapperAbstract implements DataMapperInterface
 
                 if ($column['name'] !== $conditional['dst']) {
                     $tValue = $property->getValue($obj);
-                    if (\stripos($column['internal'], '/') !== false) {
+                    if (\stripos($column['internal'], '/') === 0) {
-                        $path = \explode('/', $column['internal']);
+                        $tValue = ArrayUtils::getArray($column['internal'], $tValue, '/');
-
-                        \array_shift($path);
-                        $path   = \implode('/', $path);
-                        $tValue = ArrayUtils::getArray($path, $tValue, '/');
                     }
+
                     $value = self::parseValue($column['type'], $tValue);
 
                     $query->set([$table . '.' . $column['name'] => $value]);
@@ -1538,26 +1538,33 @@ class DataMapperAbstract implements DataMapperInterface
             }
 
             $path = $column['internal'];
-            if (\stripos($column['internal'], '/') !== false) {
+            if (\stripos($column['internal'], '/') === 0) {
-                $path = \explode('/', $column['internal']);
+                $path = \ltrim($column['internal'], '/');
-
-                \array_shift($path); // todo: why am I doing this?
-                $path = \implode('/', $path);
             }
 
-            $property = ArrayUtils::getArray($path, $obj, '/');
+            $property = ArrayUtils::getArray($column['internal'], $obj, '/');
 
             if (isset(static::$ownsOne[$path])) {
                 $id    = self::updateOwnsOneArray($column['internal'], $property, $relations, $depth);
                 $value = self::parseValue($column['type'], $id);
 
-                // todo: should not be done if the id didn't change. but for now don't know if id changed
+                /**
+                 * @todo Orange-Management/phpOMS#232
+                 *  If a model gets updated all it's relations are also updated.
+                 *  This should be prevented if the relations didn't change.
+                 *  No solution yet.
+                 */
                 $query->set([static::$table . '.' . $column['name'] => $value]);
             } elseif (isset(static::$belongsTo[$path])) {
                 $id    = self::updateBelongsToArray($column['internal'], $property, $relations, $depth);
                 $value = self::parseValue($column['type'], $id);
 
-                // todo: should not be done if the id didn't change. but for now don't know if id changed
+                /**
+                 * @todo Orange-Management/phpOMS#232
+                 *  If a model gets updated all it's relations are also updated.
+                 *  This should be prevented if the relations didn't change.
+                 *  No solution yet.
+                 */
                 $query->set([static::$table . '.' . $column['name'] => $value]);
             } elseif ($column['name'] !== static::$primaryField) {
                 $value = self::parseValue($column['type'], $property);
@@ -1590,15 +1597,7 @@ class DataMapperAbstract implements DataMapperInterface
                 ->where($table . '.' . $conditional['dst'], '=', $objId);
 
             foreach ($conditional['columns'] as $key => $column) {
-                $path = $column['internal'];
+                $property = ArrayUtils::getArray($column['internal'], $obj, '/');
-                if (\stripos($column['internal'], '/') !== false) {
-                    $path = \explode('/', $column['internal']);
-
-                    \array_shift($path); // todo: why am I doing this?
-                    $path = \implode('/', $path);
-                }
-
-                $property = ArrayUtils::getArray($path, $obj, '/');
 
                 if ($column['name'] !== $conditional['dst']) {
                     $value = self::parseValue($column['type'], $property);
@@ -1762,8 +1761,11 @@ class DataMapperAbstract implements DataMapperInterface
                     continue;
                 }
 
-                // todo: could be a problem, relation needs to be removed first?!
+                /**
-
+                 * @todo Orange-Management/phpOMS#233
+                 *  On delete the relations and relation tables need to be deleted first
+                 *  The exception is of course the belongsTo relation.
+                 */
             }
 
             self::deleteRelationTable($propertyName, $objsIds, $objId);
@@ -1880,9 +1882,11 @@ class DataMapperAbstract implements DataMapperInterface
                     $property->setAccessible(true);
                 }
 
-                // todo: the order of deletion could be a problem. maybe looping through ownsOne and belongsTo first is better.
+                /**
-                // todo: support other relation types as well (belongsto, ownsone) = for better control
+                 * @todo Orange-Management/phpOMS#233
-
+                 *  On delete the relations and relation tables need to be deleted first
+                 *  The exception is of course the belongsTo relation.
+                 */
                 foreach (static::$columns as $key => $column) {
                     if ($relations === RelationType::ALL && isset(static::$ownsOne[$propertyName]) && $column['internal'] === $propertyName) {
                         self::deleteOwnsOne($propertyName, $property->getValue($obj));
@@ -2015,8 +2019,11 @@ class DataMapperAbstract implements DataMapperInterface
         }
 
         if (!isset($obj)) {
-            // todo: implement solution for classes with constructor arguments
+            /**
-            // maybe implement a factory pattern for every datamapper model
+             * @todo Orange-Management/phpOMS#67
+             *  Since some models require special initialization a model factory should be implemented.
+             *  This could be a simple initialize() function in the mapper where the default initialize() is the current defined empty initialization in the DataMapperAbstract.
+             */
             $obj = new $class();
         }
 

Stdlib/Graph/BinaryTree.php

@@ -49,11 +49,11 @@ class BinaryTree extends Tree
      *
      * @param Node $base Tree node
      *
-     * @return Node Left node
+     * @return null|Node Left node
      *
      * @since 1.0.0
      */
-    public function getLeft(Node $base)
+    public function getLeft(Node $base) : ?Node
     {
         $neighbors = $base->getNeighbors($base);
 
@@ -66,11 +66,11 @@ class BinaryTree extends Tree
      *
      * @param Node $base Tree node
      *
-     * @return Node Right node
+     * @return null|Node Right node
      *
      * @since 1.0.0
      */
-    public function getRight(Node $base)
+    public function getRight(Node $base) : ?Node
     {
         $neighbors = $this->getNeighbors($base);
 
@@ -198,8 +198,8 @@ class BinaryTree extends Tree
     /**
      * Check if tree is symmetric.
      *
-     * @param Node $node1 Tree node1
+     * @param null|Node $node1 Tree node1
-     * @param Node $node2 Tree node2 (optional, can be different tree)
+     * @param null|Node $node2 Tree node2 (optional, can be different tree)
      *
      * @return bool True if tree is symmetric, false if tree is not symmetric
      *
@@ -207,21 +207,20 @@ class BinaryTree extends Tree
      */
     public function isSymmetric(Node $node1 = null, Node $node2 = null) : bool
     {
-        if ($node1 === null && $node2 === null) {
+        if (($node1 === null && $node2 === null)
+            || $node1->isEqual($node2)
+        ) {
             return true;
+        } elseif ($node1 === null || $node2 === null) {
+            return false;
         }
 
         $left1  = $this->getLeft($node1);
         $right1 = $this->getRight($node1);
 
-        $left2  = $node2 !== null ? $this->getLeft($node1) : $this->getLeft($node2);
+        $left2  = $node2 !== null ? $this->getLeft($node1) : null;
-        $right2 = $node2 !== null ? $this->getRight($node1) : $this->getRight($node2);
+        $right2 = $node2 !== null ? $this->getRight($node1) : null;
 
-        // todo: compare values? true symmetry requires the values to be the same
+        return $this->isSymmetric($left1, $right2) && $this->isSymmetric($right1, $left2);
-        if ($node1 !== null && $node2 !== null) {
-            return $this->isSymmetric($left1, $right2) && $this->isSymmetric($right1, $left2);
-        }
-
-        return false;
     }
 }

Stdlib/Graph/Node.php

@@ -67,4 +67,18 @@ class Node
     {
         $this->data = $data;
     }
+
+    /**
+     * Compare with other node.
+     *
+     * @param Node $node Node
+     *
+     * @return boll
+     *
+     * @since 1.0.0
+     */
+    public function isEqual(Node $node) : bool
+    {
+        return true;
+    }
 }

Utils/TaskSchedule/Schedule.php

@@ -39,7 +39,10 @@ class Schedule extends TaskAbstract
      */
     public static function createWith(array $jobData) : TaskAbstract
     {
-        // todo: fix interval this is just a dummy|!!!
+        /**
+         * @todo Orange-Management/phpOMS#231
+         *  Use the interval for generating a schedule
+         */
         $job = new self($jobData[1], $jobData[8], 'asdf');
 
         $job->setStatus($jobData[3]);