/**
   * Implementors are expected to return a count of IDs available to be migrated.
   *
   * @API: Commented out so not to break implementations which haven't implemented
   *  this yet - require it in the next API.
   *
   * @return int
   */
  //abstract public function computeCount();

  /**
   * Deprecated - implement computeCount() instead
   */
  //abstract public function count($refresh = TRUE);

And at runtime I get this error:

Fatal error: Call to undefined method MyListImplementation::count()

What should I do here? The documentation is seriously misleading! I will read the code and find out the truth, but you should definitely fix this broken implementation and documentation.

Any explaination is welcome, thanks!

Comments

pounard’s picture

pounard
Primary language French
commented

Found out:

  public function computeCount() {
    // @API: Support old count method for now.
    if (method_exists($this->listClass, 'computeCount')) {
      return $this->listClass->computeCount();
    }
    else {
      return $this->listClass->count();
    }
  }

You are keeping the count() method for backward compatibility but supporting the new one by defining computeCount(), you are trying to maintain two different and conflicting API versions in the same code, which is a definite FTL (For The Fail) because the code is not self-documenting (no method signature is declared) and you are forced to do runtime checks.

You may be able to drop either one of those and make the mandatory one abstract, or at least provide a default implementation without breaking it, but I don't see a clean solution here.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Version: 7.x-2.2 » 7.x-2.x-dev
Category: support » task

OK, as long as we're doing some API breakage in the file area, maybe it's time to remove some long-deprecated stuff like this... I would hope by now any active custom source plugins should be using the current preferred computeCount method.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Active » Fixed

OK, cleaned this up along with some other deprecated stuff.

pounard’s picture

pounard
Primary language French
commented

Simple question, why aren't you using just count() here, using the Countable SPL interface, it would make sense?

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented

count() is implemented in MigrateSource, and manages caching the counts. The computeCount() in the individual source classes is responsible for getting the real current count, and is called by count() only when actually needed.

pounard’s picture

pounard
Primary language French
commented

Oh right ok, but there is no name colliding even keeping the Countable interface for both, right?

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.