Можно ли упростить этот код (для циклов) для поиска дубликатов, учитывая множество условий?

ProgramBox

Можно ли упростить этот код (для циклов) для поиска дубликатов, учитывая множество условий?

Post author:admin
Запись опубликована:12 апреля, 2023
Post category:Вопросы по программированию

#visual-c #mfc

Вопрос:

Я вырезал часть кода здесь, чтобы просто показать соответствующий бит:

 void CImportOCLMAssignmentHistoryDlg::SetButtonStates()
{
    {
        // Don't allow import into the same assignment more than once.
        CByteArray aryImport;
        aryImport.SetSize(15);
        aryImport[0] = m_iImportOCLMHost;
        aryImport[1] = m_iImportOCLMCohost;
        aryImport[2] = m_iImportOCLMChairman;
        aryImport[3] = m_iImportOCLMOpenPrayer;
        aryImport[4] = m_iImportOCLMClosePrayer;
        aryImport[5] = m_iImportOCLMConductorCBS;
        aryImport[6] = m_iImportOCLMReaderCBS;
        aryImport[7] = m_iImportPTHost;
        aryImport[8] = m_iImportPTCohost;
        aryImport[9] = m_iImportPTChairman;
        aryImport[10] = m_iImportPTHospitality;
        aryImport[11] = m_iImportWTConductor;
        aryImport[12] = m_iImportWTReader;
        aryImport[13] = m_iImportPTSpeaker; // AJT v17.0.8
        aryImport[14] = m_iImportPTTheme;   // AJT v17.0.8

        COptionsDlg dlgOptions;
        REPORT_MODE_E eMode = dlgOptions.GetReportMode();
        bool bDuplicateAssignment = false;

        if (eMode == MODE_MEETING)
        {
            // The index values can't be duplicated within each meeting
            // Midweek 0 to 6
            // Weekend 7 to 14

            // Midweek
            for (int iAssignment = 0; iAssignment < 7; iAssignment  )
            {
                if (aryImport[iAssignment] != (BYTE)-1)
                {
                    for (int iAssignment2 = iAssignment   1; iAssignment2 < 7; iAssignment2  )
                    {
                        if (aryImport[iAssignment2] == aryImport[iAssignment])
                        {
                            bDuplicateAssignment = true;
                            break;
                        }
                    }
                    if (bDuplicateAssignment)
                        break;
                }
            }

            if (!bDuplicateAssignment)
            {
                // Weekend
                for (int iAssignment = 7; iAssignment < aryImport.GetSize(); iAssignment  )
                {
                    if (aryImport[iAssignment] != (BYTE)-1)
                    {
                        for (int iAssignment2 = iAssignment   1; iAssignment2 < aryImport.GetSize(); iAssignment2  )
                        {
                            if (aryImport[iAssignment2] == aryImport[iAssignment])
                            {
                                bDuplicateAssignment = true;
                                break;
                            }
                        }
                        if (bDuplicateAssignment)
                            break;
                    }
                }
            }
        }
        else
        {
            // The index values can't be duplicated anywhere in the array
            for (int iAssignment = 0; iAssignment < aryImport.GetSize(); iAssignment  )
            {
                if (aryImport[iAssignment] != (BYTE)-1)
                {
                    for (int iAssignment2 = iAssignment   1; iAssignment2 < aryImport.GetSize(); iAssignment2  )
                    {
                        if (aryImport[iAssignment2] == aryImport[iAssignment])
                        {
                            bDuplicateAssignment = true;
                            break;
                        }
                    }
                    if (bDuplicateAssignment)
                        break;
                }
            }
        }

        if (bDuplicateAssignment)
            m_btnImport.EnableWindow(FALSE);
        else
            m_btnImport.EnableWindow(TRUE);
    }
}

Код работоспособен. Мне просто интересно, можно ли его упростить без чрезмерного усложнения удобства чтения.
Нет необходимости комментировать мое использование литеральных целых чисел вместо #define или enum значений. Я знаю об этом улучшении. Это в первую очередь способ, которым я ищу дубликаты. Можно ли это упростить?

1. Возможно, это еще один вопрос для проверки кода? Однако быстрый просмотр первого из ваших for циклов указывает только на одно отличие: изменение между 7 и aryImport.GetSize() ; таким образом, присвоение одного из этих значений ‘новой’ переменной сделало бы циклы идентичными … или я что-то пропустил?

2. … и вместо этого дополнительный материал в else блоке можно было бы просто переместить в if (eMode != MODE_MEETING) блок.

3. @AdrianMole Я понимаю, что мог бы написать вторую функцию, которой передается массив байтов и диапазон начала / конца для сравнения. Мне просто интересно, можно ли что-то сделать внутри самой функции.

4. Конечно: Создайте int limit = 7; и затем if (eMode == MODE_MEETING) limit = aryImport.GetSize(); используйте limit в for циклах (которые в остальном идентичны).

5. for Цикл в коде C является убедительным признаком того, что вы забыли #include <algorithm> . Поиск дубликатов можно заменить на сортировку , за которой следует adjacent_find . Если ваш контейнер уже отсортирован, вы можете пропустить первый шаг.

Ответ №1:

Вместо двух вложенных проходов через ваш массив вы можете использовать bitmap для проверки на наличие дубликатов:

 bool CImportOCLMAssignmentHistoryDlg::DetectDuplicateAssignment(CByteArray amp;aryImport, INT_PTR iMin, INT_PTR iMax)
{
  std::vector<bool> found(256);
  for (INT_PTR iAssignment = iMin; iAssignment < iMax; iAssignment  )
  {
    auto value = aryImport[iAssignment];
    if (value != (BYTE)-1)
    {
      if (found[value])
        return true;
      found[value] = true;
    }
  }
  return false;
}

Чтобы сделать его более похожим на C , используйте std::vector<bool> found(256); вместо массива в стиле C; также экономится некоторое пространство.

1. std::vector<bool> может быть оптимизировано пространство, а может и не быть. И, скорее всего, это контейнер C (если вы хотите его так называть) с самым неожиданным поведением. Лучшим выбором здесь было бы std::bitset .

Ответ №2:

На данный момент я сделал это следующим образом:

 void CImportOCLMAssignmentHistoryDlg::SetButtonStates()
{
    {
        // Don't allow import into the same assignment more than once.
        CByteArray aryImport;
        aryImport.SetSize(15);
        aryImport[0] = m_iImportOCLMHost;
        aryImport[1] = m_iImportOCLMCohost;
        aryImport[2] = m_iImportOCLMChairman;
        aryImport[3] = m_iImportOCLMOpenPrayer;
        aryImport[4] = m_iImportOCLMClosePrayer;
        aryImport[5] = m_iImportOCLMConductorCBS;
        aryImport[6] = m_iImportOCLMReaderCBS;
        aryImport[7] = m_iImportPTHost;
        aryImport[8] = m_iImportPTCohost;
        aryImport[9] = m_iImportPTChairman;
        aryImport[10] = m_iImportPTHospitality;
        aryImport[11] = m_iImportWTConductor;
        aryImport[12] = m_iImportWTReader;
        aryImport[13] = m_iImportPTSpeaker; // AJT v17.0.8
        aryImport[14] = m_iImportPTTheme;   // AJT v17.0.8

        COptionsDlg dlgOptions;
        REPORT_MODE_E eMode = dlgOptions.GetReportMode();
        bool bDuplicateAssignment = false;

        if (eMode == MODE_MEETING)
        {
            // The index values can't be duplicated within each meeting
            // Midweek 0 to 6
            // Weekend 7 to 14

            // Midweek
            bDuplicateAssignment = DetectDuplicateAssignment(aryImport, 0, 7);

            if (!bDuplicateAssignment)
            {
                // Weekend
                bDuplicateAssignment = DetectDuplicateAssignment(aryImport, 7, aryImport.GetSize());
            }
        }
        else
        {
            // The index values can't be duplicated anywhere in the array
            bDuplicateAssignment = DetectDuplicateAssignment(aryImport, 0, aryImport.GetSize());
        }

        if (bDuplicateAssignment)
            m_btnImport.EnableWindow(FALSE);
        else
            m_btnImport.EnableWindow(TRUE);
    }
}

bool CImportOCLMAssignmentHistoryDlg::DetectDuplicateAssignment(CByteArray amp;aryImport, INT_PTR iMin, INT_PTR iMax)
{
    bool bDuplicateAssignment = false;

    for (INT_PTR iAssignment = iMin; iAssignment < iMax; iAssignment  )
    {
        if (aryImport[iAssignment] != (BYTE)-1)
        {
            for (INT_PTR iAssignment2 = iAssignment   1; iAssignment2 < iMax; iAssignment2  )
            {
                if (aryImport[iAssignment2] == aryImport[iAssignment])
                {
                    bDuplicateAssignment = true;
                    break;
                }
            }
            if (bDuplicateAssignment)
                break;
        }
    }

    return bDuplicateAssignment;
}

Если у кого-нибудь есть альтернативный подход, пожалуйста, дайте мне знать. Спасибо.