Здравствуйте, mgu, Вы писали:

mgu>Спасибо за полезную ссылку, но вот что меня беспокоит: при вашей политике распространения библиотеки в виде исходного кода, как бедные пользователи будут угадывать правильный компилятор?

Гораздо важнее другой вопрос: когда библиотека поставляется в бинарном виде, как быть тогда? На ваш же вопрос ответ прост: поддержка С++11.

mng>И что им делать, если их текущий сборщик не переварит всех нововведений?

Искать что-нибудь более подходящее для себя. Написать свое. Заплатить за адаптацию под свои условия. Переделать самостоятельно. С исходным текстом можно делать все, что угодно.

S>>Так что вы придрались к синтаксису.

mgu>Да нет, к подходу -- налицо излишняя многословность.

Разница только в количестве строк.

mgu> Против инкрементов у вас же возражений нет?

Инкременты откуда взялись?

S>>К коду AlexGin сходу можно предъявлять претензии гораздо более серьезные.

mgu>Например?

Да вот, первый же cpp-файл в репозитории. И сразу же:

bool SMSContentGreater(CSMSContent* pSMS1, CSMSContent* pSMS2) // Global function for std::sort
{
    CTime time1, time2;
    SYSTEMTIME st1, st2;
    if (pSMS1->GetSMSTime().GetAsSystemTime(st1))
        time1 = CTime(st1);
    if (pSMS2->GetSMSTime().GetAsSystemTime(st2))
        time2 = CTime(st2);

    time_t t1 = time1.GetTime();
    time_t t2 = time2.GetTime();

    if (t1 > t2)
        return true;
    else
        return false;
}

Здесь есть две последовательно выполняемые независимые операции -- извлечение времени в виде SYSTEMTIME и трансформация его в time_t через промежуточный CTime. Даже если предположить, что столь длинная цепочка нужна, то все равно непонятно, зачем нужны две пары переменных time1+time2 и st1+st2. Если можно обойтись всего одной переменной типа SYSTEMTIME и одной переменной типа CTime. Что-то вроде:

CTime ct;
SYSTEMTIME st;
if(pSMS1->GetSMSTime().GetAsSystemTime(st))
  ct = CTime(st);
time_t t1 = ct.GetTime();

if(pSMS2->GetSMSTime().GetAsSystemTime(st))
  ct = CTime(st);
time_t t2 = ct.GetTime();

Здесь становится очевидно, что имеется две однотипных операции и явная копи-паста. Которая устраняется через вспомогательную функцию:

namespace {
inline time_t GetTime(const CSMSContent & sms) {
  SYSTEMTIME st;
  return sms.GetSMSTime().GetAsSytemTime(st)) ? CTime(st).GetTime() : CTime().GetTime();
}
}

bool SMSContentGreater(CSMSContent * pSMS1, CSMSContent * pSMS2) {
  return GetTime(*pSMS1) > GetTime(*pSMS2);
}

И это все в рамках C++98/03, что несколько многословно. Можно воспользоваться возможностями не очень уже современного C++11 и имитацией локальных функций в нем:

bool SMSContentGreater(CSMSContent * pSMS1, CSMSContent * pSMS2) {
  auto GetTime = [](const CSMSContent & sms) {
    SYSTEMTIME st;
    return sms.GetSMSTime().GetAsSytemTime(st)) ? CTime(st).GetTime() : CTime().GetTime();
  };
  return GetTime(*pSMS1) > GetTime(*pSMS2);
}

Однако, особая пикантность ситуации состоит в том, что у CTime уже есть оператор "меньше". Посему непонятно, а зачем вообще нужна еще одна трансформация из CTime в time_t. Более того, очень похоже, что GetSMSTime уже возвращает объект типа CTime, у которого затем дергается GetAsSystemTime() дабы получить SYSTEMTIME. А затем из SYSTEMTIME вновь конструируется CTime... Чой-то неведомое.

И это только первая же функция в первом же файле. Смотрим этот же файл дальше:

CATCmdDevice::CATCmdDevice()
: m_pSerial(NULL), // Added 17.03.2015
  m_bIsGSMDeviceConnected(false) // Added 21.03.2015
{
    m_pTXBuff = NULL;  // Transmitter buffer
    m_pRXBuff = NULL;  // Reciver buffer

    m_bUnicodeMode = false;

    m_pPBRecord = NULL;

    m_bReceivedSMSListFull = false;
}

Конструктор. Только часть полей инициализируется через списки инициализации, часть -- в теле конструктора. Причем та часть, которая в теле конструктора, должна была бы инициализироваться в списках инициализации еще со времен C++98.

Дальше, деструктор:

CATCmdDevice::~CATCmdDevice()
{
    ClearSMSArr(); // Restored 01.10.2014

    if (m_pTXBuff)
    {
        delete[] m_pTXBuff;  
        m_pTXBuff = NULL;
    } 

    if (m_pRXBuff)
    {
        delete[] m_pRXBuff;  
        m_pRXBuff = NULL;
    }
}

Поля m_pTXBuff и m_pRXBuff объявлены в самом классе CATCmdDevice. Следовательно, занулять их в деструкторе после освобождения памяти смысла нет.

Опять же, на счет m_pTXBuff и m_pRXBuff: почему идет ручное управление памятью? Почему нельзя было воспользоваться std::vector (рекомендуемый путь со времен C++98)? Или вообще почему не сделать m_TXBuff и m_RXBuff обычными C-шными массивами?

Далее:

long CATCmdDevice::OpenSerial(int iComChannel, int iComBaudRate)
{
    if (m_pSerial)
        delete m_pSerial;

    m_pSerial = new CSerial();
    if (m_pSerial != NULL)
    {   // m_pSerial->Open(1, 9600);
        BOOL bOpen = m_pSerial->Open(iComChannel, iComBaudRate); 
        if(!bOpen)
        {
            g_log.SaveLogFile(PBK_ERROR, "Error during opening port: COM%u (baud rate = %u)",iComChannel,iComBaudRate);
            return 1L; // Error occur
        }
        else
            g_log.SaveLogFile(PBK_DEBUG, "Port: COM%u (baud rate = %u) - opening success!",iComChannel,iComBaudRate);
    }
    return 0L; // OK!
}

Что за проверка m_pSerial != NULL после new? Неужели автор транслирует свой код с запретом C++ных исключений на уровне ключей компилятора? Если даже он делает именно так, то где ветка else у этого if-а? А то ведь получается, что при запрещенных исключениях у него нет обработки ошибки выделения памяти (даже если в m_pSerial окажется 0, функция OpenSerial все равно вернет 0 в качестве признака успешного завершения). Если же исключения не запрещены, то данная проверка не имеет смысла, т.к. при недостатке памяти new бросит bad_alloc.

И это, повторюсь, всего лишь беглый просмотр всего лишь первого файла.

Несомненное достоинство данного кода -- это возможность в нем разобраться. Но общее впечатление, что его писал вчерашний студент, который только-только начал изучать язык C++.

По остальным вопросам в следующем ответе.