2

I am attempting to loop through my "journalEntries" in order to find duplicates. For some reason, it is taking a very long time to finish and return a response to me. Last time I tested it, it took 12 seconds to finish.

$criteria = craft()->elements->getCriteria(ElementType::Entry);
$criteria->section = 'journalArticles';
$criteria->limit = 10;
$copies = array();

for($a = 0; $a < count($criteria); $a++) {
  for($b = $a + 1; $b < count($criteria); $b++) {
    if($criteria[$a]->title === $criteria[$b]->title) {
      array_push($copies, $criteria[$a]->title);
    }
  }
}

$this->returnJson($copies);

I think it is something I am doing incorrectly within the two for loops. As you can see, I limit the amount of articles to only 10, so something seriously funky must be going on. Any help?

Improve this question
3

Something like this should work and be faster.

$criteria = craft()->elements->getCriteria(ElementType::Entry);
$criteria->section = 'journalArticles';
$criteria->limit = 10;
$entries = $criteria->find();

$titles = array();
$copies = array();

foreach ($entries as $entry)
{
    $titles[] = $entry->title;
}

foreach (array_count_values($titles) as $val => $count)
{
    if ($count > 1)
    {
        $copies[] = $val;
    }
}

$this->returnJson($copies);
Improve this answer
1
  • This is legit. I have it working now. Thanks a lot for your help. – Dan Zuzevich Dec 27 '17 at 18:51
3

Just some side notes as an explanation why your code takes so long and to understand Craft a little bit more your count() function does actually this:

public function count()
{
    // If the query has already been executed, just return a count of the results.
    if (isset($this->_matchedElements))
    {
        return count($this->_matchedElements);
    }

    $total = $this->total();

    if ($this->offset)
    {
        $total -= $this->offset;

        if ($total < 0)
        {
            $total = 0;
        }
    }

    if ($this->limit && $total > $this->limit)
    {
        $total = $this->limit;
    }

    return $total;
}

So it checks if there are cached elements and if not the total function is executed

public function total($attributes = null)
{
    $this->setAttributes($attributes);

    $this->_includeInTemplateCaches();

    if (!isset($this->_cachedTotal))
    {
        $this->_cachedTotal = craft()->elements->getTotalElements($this);
    }

    return $this->_cachedTotal;
}

Which only caches the number but not the elements

So what you actually do is executing the query about 100 times (because of your two loops)

Just using $entries = $criteria->find() and looping through your entries is a far better idea

Improve this answer
5
  • I should have just waited on you to answer. :) – Brad Bell Dec 27 '17 at 18:11
  • It's good you answered first because I would have done some nasty workaround since I forgot about the array_count_values() function. With your answer he has a solid code, I just wanted to show some things behind the scene (I hope this is the correct reason why it took so long because the longer I look at the code the more I have the feeling I make a fool of myself) – Robin Schambach Dec 27 '17 at 18:24
  • Testing the code out now ~_~ – Dan Zuzevich Dec 27 '17 at 18:38
  • Thanks again for diving deep and explaining whats going on behind the scenes. So what your saying is that since I did not use the find method, and set that result to a variable, I am querying the database for the elements 100 times? Holy crapola. – Dan Zuzevich Dec 27 '17 at 18:54
  • @RobinSchambach Nope, I think you're right... the first example was basically O(n²) algorithm, so with 10 inputs you get 100 executions. – Brad Bell Dec 27 '17 at 19:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.