microsoft / GSL

Guidelines Support Library
Other
6.14k stars 738 forks source link

Upgrading code related to the CMonthCalCtrl control #1010

Closed ajtruckle closed 2 years ago

ajtruckle commented 2 years ago

I have a few issues here concerning the CMonthCalCtrl control so I thought I would collate them all together in one issue ticket.

BOLDDAY

Most of it is related to setting day states (making them bold) and the concept is described in detail here. You have to define a macro:

#define  BOLDDAY(ds, iDay) if(iDay > 0 && iDay < 32) \
            (ds) |= (0x00000001 << (iDay-1))

This macro is used in the following function:

void CMeetingScheduleAssistantDlg::InitDayStateArray(int iMonthCount, LPMONTHDAYSTATE pDayState, COleDateTime datStart)
{
    int                 iMonth = 0;
    COleDateTimeSpan    spnDay;
    CString             strKey;
    SPECIAL_EVENT_S     *psEvent = nullptr;

    if (pDayState == nullptr)
        return;

    memset(pDayState, 0, sizeof(MONTHDAYSTATE)*iMonthCount);

    if (m_pMapSPtrEvents == nullptr && m_Reminders.Count() == 0)
    {
        return;
    }

    spnDay.SetDateTimeSpan(1, 0, 0, 0);

    auto datDay = datStart;
    const auto iStartMonth = datStart.GetMonth();
    auto iThisMonth = iStartMonth;
    auto iLastMonth = iThisMonth;
    do
    {
        strKey = datDay.Format(_T("%Y-%m-%d"));

        if (m_pMapSPtrEvents != nullptr)
        {
            psEvent = nullptr;
            m_pMapSPtrEvents->Lookup(strKey, reinterpret_cast<void*&>(psEvent));
            if (psEvent != nullptr)
            {
                BOLDDAY(pDayState[iMonth], datDay.GetDay());
            }
        }

        if (m_Reminders.HasReminder(datDay))
        {
            BOLDDAY(pDayState[iMonth], datDay.GetDay());
        }

        datDay = datDay + spnDay;
        iThisMonth = datDay.GetMonth();
        if (iThisMonth != iLastMonth)
        {
            iLastMonth = iThisMonth;
            iMonth++;
        }
    } while (iMonth < iMonthCount);
}

Everywhere I use this BOLDDAY macro I get a code analysis warning (C26481):

Warning3

It is not clear to me if the problem is with the BOLDDAY macro or my own code?

Collection Query 1

Then, I have a related query about the collection of MONTHDAYSTATE objects. For example:

void CMeetingScheduleAssistantDlg::SetDayStates(CMonthCalCtrl &rCalendar)
{
    COleDateTime        datFrom, datUntil;

    auto iMonthCount = rCalendar.GetMonthRange(datFrom, datUntil, GMR_DAYSTATE);
    auto pDayState = new MONTHDAYSTATE[iMonthCount];
    if (pDayState != nullptr)
    {
        InitDayStateArray(iMonthCount, pDayState, datFrom);
        VERIFY(rCalendar.SetDayState(iMonthCount, pDayState));
        delete[] pDayState;
    }
}

If I turn pDayState into: auto pDayState = std::make_unique<MONTHDAYSTATE>(iMonthCount); I am then not sure how to proceed:

  1. I know I could pass to InitDayStateArray the parameter pDayState.release() but then it seems little point to do that because we will need to clean up the memory. So this is a no no.
  2. I know I could pass to InitDayStateArray the parameter pDayState.get() but it is not clear to me if that will be the correct way forward.
  3. I also wonder if the logic of InitDayStateArray can now be simplified if we just pass the new smart pointer?

I am sorry if these are too many questions in one and I am happy to split if required.

Collection Query 2

I have a couple of event handlers. Here is an example:

void CMeetingScheduleAssistantDlg::OnGetDayStateStart(NMHDR* pNMHDR, LRESULT* pResult)
{
    NMDAYSTATE      *pDayState = reinterpret_cast<NMDAYSTATE*>(pNMHDR);
    MONTHDAYSTATE   mdState[3]{}; // 1 = prev 2 = curr 3 = next
    const COleDateTime  datStart(pDayState->stStart);

    if (pDayState != nullptr)
    {
        InitDayStateArray(pDayState->cDayState, &mdState[0], datStart);
        pDayState->prgDayState = &mdState[0];
    }

    if(pResult != nullptr)
        *pResult = 0;
}

Again, it is similar is the sense that we end up calling the same method, but the way we go about it is slightly different.

In Summary

beinhaerter commented 2 years ago

Your code is not using MS-GSL. So I (just a contributor, not a maintainer) don't understand why you are opening a GSL issue. This is an issue with your code not with MS-GSL. In my opinion your question(s) is/are better suited at stackoverflow.com or codereview.stackexchange.com. I appreciate that you modernize your code and I am willing to help, so if you ask a question on the mentioned sites feel free to address me there ("@WernerHenze" in a comment).

ajtruckle commented 2 years ago

@beinhaerter All valid points and I agree 100%. I also appreciate your willingness to help. I'll prepare the first question now.