Simplified constructor for LinkedVersions collection and also simplif…

…ied isAcceptable()
gsomoza · Jul 13, 2015 · 243e1f4 · 243e1f4
1 parent dc05241
commit 243e1f4
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 31 deletions.
diff --git a/lib/Version/Collection/IndexedVersions.php b/lib/Version/Collection/IndexedVersions.php
@@ -56,18 +56,13 @@ public function __construct($versions = array())
      * Returns true if the specified version is valid (can be added) to the collection. Otherwise, it MUST throw
      * an exception.
      *
-     * @param mixed $version
+     * @param Version $version
      * @return bool
      * @throws InvalidArgumentException
      */
-    public function isAcceptable($version)
+    public function isAcceptable(Version $version)
     {
-        if (!is_object($version) || !$version instanceof Version) {
-            throw new InvalidArgumentException(
-                sprintf('Expected version to be of type "%s"', Version::class)
-            );
-        }
-        return true;
+        return $version; // always true
     }
 
     /**

diff --git a/lib/Version/Collection/LinkedVersions.php b/lib/Version/Collection/LinkedVersions.php
@@ -33,20 +33,19 @@ class LinkedVersions extends SortableVersions
     /**
      * Validates that migrations added to this set must all have a linked Migration
      *
-     * @param mixed $version
+     * @param Version $version
      * @return bool
      *
      * @throws MigrationMissingException
      */
-    public function isAcceptable($version)
+    public function isAcceptable(Version $version)
     {
-        /** @var Version $version */
-        if (parent::isAcceptable($version) && !$version->hasMigration()) {
+        if (!$version->hasMigration()) {
             throw new MigrationMissingException(sprintf(
                 'Version "%s" must be associated with a Migration in order to be accepted into this collection.',
                 $version->getId()
             ));
         }
-        return true;
+        return parent::isAcceptable($version);
     }
 }
diff --git a/lib/Version/Collection/MigratedVersions.php b/lib/Version/Collection/MigratedVersions.php
@@ -29,20 +29,19 @@ class MigratedVersions extends SortableVersions
     /**
      * This makes the collection behave like a set - throwing an exception if the version already exists in the set.
      *
-     * @param mixed $version
+     * @param Version $version
      * @return bool
      * @throws CollectionException
      * @throws MigrationMissingException
      */
-    public function isAcceptable($version)
+    public function isAcceptable(Version $version)
     {
-        /** @var Version $version */
-        if (parent::isAcceptable($version) && !$version->isMigrated()) {
+        if (!$version->isMigrated()) {
             throw new CollectionException(sprintf(
                 'Version "%s" must be migrated in order to be accepted into this collection.',
                 $version->getId()
             ));
         }
-        return true;
+        return parent::isAcceptable($version);
     }
 }
diff --git a/lib/Version/Collection/SortableVersions.php b/lib/Version/Collection/SortableVersions.php
@@ -35,20 +35,19 @@ class SortableVersions extends IndexedVersions
     /**
      * This makes the collection behave like a set - throwing an exception if the version already exists in the set.
      *
-     * @param mixed $version
+     * @param Version $version
      * @return bool
      *
      * @throws CollectionException
      */
-    public function isAcceptable($version)
+    public function isAcceptable(Version $version)
     {
-        /** @var Version $version */
-        if (parent::isAcceptable($version) && $this->has($version->getId())) {
+        if ($this->has($version->getId())) {
             throw new CollectionException(
                 sprintf('Item with id "%s" already exists', $version->getId())
             );
         }
-        return true;
+        return parent::isAcceptable($version);
     }
 
     /**

diff --git a/test/Version/CollectionTest.php b/test/Version/CollectionTest.php
@@ -53,14 +53,6 @@ public function testConstructorIterator()
         $this->assertCount(3, $instance);
     }
 
-    public function testConstructorWithInvalidVersions()
-    {
-        $versions = [new V('1'), '2', 3, new \stdClass()]; // only first one is valid
-
-        $this->setExpectedException(\Exception::class);
-        $instance = new SortableVersions($versions);
-    }
-
     public function testConstructor()
     {
         $instance = new SortableVersions();