michaelh added a comment.

  While reading in IDE realized `(*it)` resolves to `PostingIterator**` I got a 
little dizzy at first, then started to play with this code and came up with 
this.
  In Constructor:
  
    for (PostingIterator* it : m_iterators) {
      if (it == nullptr) {
        m_iterators.erase(&it);
       } else {
         auto docId = it->next();
         if (docId && (docId < m_nextId || m_nextId == 0)) {
           m_nextId = docId;
         }
      }
    }
  
  In `OrPostingIterator::next()`
  
    for (PostingIterator* it : m_iterators) {
        auto docId = it->docId();
        if (docId <= m_docId) {
            docId = it->next();
            if (docId == 0) {
                m_iterators.erase(&it);
                continue;
            }
        }
        m_nextId = m_nextId ? qMin(docId , m_nextId) : docId;
    }
  
  I'm not 100% sure if this does exactly the same. But if it does, for readers 
on my skill level this code will be easier to understand, I guess. 
  BTW, `&it` also makes me dizzy :-)

INLINE COMMENTS

> orpostingiterator.cpp:28
>      , m_docId(0)
> +    , m_nextId(0)
>  {

`, m_nextId(ULONG_LONG_MAX)` ?

> orpostingiterator.cpp:44
> +            m_nextId = docId;
> +        }
> +

`m_nextId = (docId > 0) ? qMin(m_nextId, docId) : m_nextId;`  ?

> orpostingiterator.cpp:68
>      }
>  
> +    // advance all iterators which point to the lowest docId

Remove the above  and instead ?

  if (m_nextId == ULONG_LONG_MAX) {
    m_docId = 0;
    return 0;
  } 
  m_docId = m_nextId;

?

> orpostingiterator.cpp:71
> +    for (auto it = m_iterators.begin(); it != m_iterators.end(); ) {
>          PostingIterator* iter = *it;
>  

Newbie question: Does std::unique_ptr make sense here?

> orpostingiterator.cpp:88
> +            m_nextId = docId;
>          }
>  

`m_nextId = qMin(docId, m_nextId);` ?

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D11828

To: bruns, #baloo, michaelh
Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, 
alexeymin

Reply via email to