Cleanup some CPDFSDK_PageView annotation code.

fpdfsdk/cba_annotiterator.cpp

Index: fpdfsdk/cba_annotiterator.cpp
diff --git a/fpdfsdk/cba_annotiterator.cpp b/fpdfsdk/cba_annotiterator.cpp
index ef9ab2ad31e59620fbac064be846f12ae746b9a8..b4c3821ca2972a85c937dc49e4e5f01e82b3b26f 100644
--- a/fpdfsdk/cba_annotiterator.cpp
+++ b/fpdfsdk/cba_annotiterator.cpp
@@ -66,25 +66,43 @@ CPDFSDK_Annot* CBA_AnnotIterator::GetPrevAnnot(CPDFSDK_Annot* pAnnot) {
   return *(--iter);
 }
 
+void CBA_AnnotIterator::CollectAnnots(std::vector<CPDFSDK_Annot*>* pArray) {
+  for (auto pAnnot : m_pPageView->GetAnnotList()) {
+    if (pAnnot->GetAnnotSubtype() == m_nAnnotSubtype &&
+        !pAnnot->IsSignatureWidget()) {
+      pArray->push_back(pAnnot);
+    }
+  }
+}
+
+CFX_FloatRect CBA_AnnotIterator::AddToAnnotsList(
+    std::vector<CPDFSDK_Annot*>& sa,
+    size_t idx) {
+  CPDFSDK_Annot* pLeftTopAnnot = sa[idx];
+  CFX_FloatRect rcLeftTop = GetAnnotRect(pLeftTopAnnot);
+  m_Annots.push_back(pLeftTopAnnot);
+  sa.erase(sa.begin() + idx);
+  return rcLeftTop;
+}
+
+void CBA_AnnotIterator::AddSelectedToAnnots(std::vector<CPDFSDK_Annot*>& sa,
+                                            std::vector<size_t>& aSelect) {
+  for (size_t i = 0; i < aSelect.size(); ++i)
+    m_Annots.push_back(sa[aSelect[i]]);
+
+  for (int i = aSelect.size() - 1; i >= 0; --i)
+    sa.erase(sa.begin() + aSelect[i]);
+}
+
 void CBA_AnnotIterator::GenerateResults() {
   switch (m_eTabOrder) {
-    case STRUCTURE: {
-      for (size_t i = 0; i < m_pPageView->CountAnnots(); ++i) {
-        CPDFSDK_Annot* pAnnot = m_pPageView->GetAnnot(i);
-        if (pAnnot->GetAnnotSubtype() == m_nAnnotSubtype &&
-            !pAnnot->IsSignatureWidget())
-          m_Annots.push_back(pAnnot);
-      }
+    case STRUCTURE:
+      CollectAnnots(&m_Annots);
       break;
-    }
+
     case ROW: {
       std::vector<CPDFSDK_Annot*> sa;
-      for (size_t i = 0; i < m_pPageView->CountAnnots(); ++i) {
-        CPDFSDK_Annot* pAnnot = m_pPageView->GetAnnot(i);
-        if (pAnnot->GetAnnotSubtype() == m_nAnnotSubtype &&
-            !pAnnot->IsSignatureWidget())
-          sa.push_back(pAnnot);
-      }
+      CollectAnnots(&sa);
 
       std::sort(sa.begin(), sa.end(), CompareByLeftAscending);

[Tom Sepez, 2016/10/05 17:59:52]

super-super-super-nit:  I'd put the blank line after this so that the collect
and sort appear on adjacent lines.

[dsinclair, 2016/10/05 18:26:56]

Done.

       while (!sa.empty()) {
@@ -97,36 +115,26 @@ void CBA_AnnotIterator::GenerateResults() {
             fTop = rcAnnot.top;
           }
         }
-        if (nLeftTopIndex >= 0) {
-          CPDFSDK_Annot* pLeftTopAnnot = sa[nLeftTopIndex];
-          CFX_FloatRect rcLeftTop = GetAnnotRect(pLeftTopAnnot);
-          m_Annots.push_back(pLeftTopAnnot);
-          sa.erase(sa.begin() + nLeftTopIndex);
-
-          std::vector<int> aSelect;
-          for (size_t i = 0; i < sa.size(); ++i) {
-            CFX_FloatRect rcAnnot = GetAnnotRect(sa[i]);
-            FX_FLOAT fCenterY = (rcAnnot.top + rcAnnot.bottom) / 2.0f;
-            if (fCenterY > rcLeftTop.bottom && fCenterY < rcLeftTop.top)
-              aSelect.push_back(i);
-          }
-          for (size_t i = 0; i < aSelect.size(); ++i)
-            m_Annots.push_back(sa[aSelect[i]]);
+        if (nLeftTopIndex < 0)
+          continue;
+
+        CFX_FloatRect rcLeftTop = AddToAnnotsList(sa, nLeftTopIndex);
 
-          for (int i = aSelect.size() - 1; i >= 0; --i)
-            sa.erase(sa.begin() + aSelect[i]);
+        std::vector<size_t> aSelect;
+        for (size_t i = 0; i < sa.size(); ++i) {
+          CFX_FloatRect rcAnnot = GetAnnotRect(sa[i]);
+          FX_FLOAT fCenterY = (rcAnnot.top + rcAnnot.bottom) / 2.0f;
+          if (fCenterY > rcLeftTop.bottom && fCenterY < rcLeftTop.top)
+            aSelect.push_back(i);
         }
+        AddSelectedToAnnots(sa, aSelect);
       }
       break;
     }
+
     case COLUMN: {
       std::vector<CPDFSDK_Annot*> sa;
-      for (size_t i = 0; i < m_pPageView->CountAnnots(); ++i) {
-        CPDFSDK_Annot* pAnnot = m_pPageView->GetAnnot(i);
-        if (pAnnot->GetAnnotSubtype() == m_nAnnotSubtype &&
-            !pAnnot->IsSignatureWidget())
-          sa.push_back(pAnnot);
-      }
+      CollectAnnots(&sa);
 
       std::sort(sa.begin(), sa.end(), CompareByTopDescending);
       while (!sa.empty()) {
@@ -142,26 +150,19 @@ void CBA_AnnotIterator::GenerateResults() {
             fLeft = rcAnnot.left;
           }
         }
+        if (nLeftTopIndex < 0)
+          continue;
 
-        if (nLeftTopIndex >= 0) {
-          CPDFSDK_Annot* pLeftTopAnnot = sa[nLeftTopIndex];
-          CFX_FloatRect rcLeftTop = GetAnnotRect(pLeftTopAnnot);
-          m_Annots.push_back(pLeftTopAnnot);
-          sa.erase(sa.begin() + nLeftTopIndex);
-
-          std::vector<int> aSelect;
-          for (size_t i = 0; i < sa.size(); ++i) {
-            CFX_FloatRect rcAnnot = GetAnnotRect(sa[i]);
-            FX_FLOAT fCenterX = (rcAnnot.left + rcAnnot.right) / 2.0f;
-            if (fCenterX > rcLeftTop.left && fCenterX < rcLeftTop.right)
-              aSelect.push_back(i);
-          }
-          for (size_t i = 0; i < aSelect.size(); ++i)
-            m_Annots.push_back(sa[aSelect[i]]);
+        CFX_FloatRect rcLeftTop = AddToAnnotsList(sa, nLeftTopIndex);
 
-          for (int i = aSelect.size() - 1; i >= 0; --i)
-            sa.erase(sa.begin() + aSelect[i]);
+        std::vector<size_t> aSelect;
+        for (size_t i = 0; i < sa.size(); ++i) {
+          CFX_FloatRect rcAnnot = GetAnnotRect(sa[i]);
+          FX_FLOAT fCenterX = (rcAnnot.left + rcAnnot.right) / 2.0f;
+          if (fCenterX > rcLeftTop.left && fCenterX < rcLeftTop.right)
+            aSelect.push_back(i);
         }
+        AddSelectedToAnnots(sa, aSelect);
       }
       break;
     }