0
\$\begingroup\$

I am inserting elements into the map container. For this I am using the same statements multiple times in the below function. Is there any way I can write a generic function for this? So that even at the later point of time, when required, I can insert new elements.

 bool EMRMgr::GetParams()
    {
        EmIOStruct emIOstructObj;
        WCHAR szValue[MAX_PATH] = { 0 };
        DWORD dwDataSize = sizeof(szValue) / sizeof(WCHAR);
        long lRes = 0;
        McStorageType mcStorageTypeObj = McStorageType::eRegistry;
        std::wstring value;

    // First time I am using the below statements to insert elements into the map container

        mcIOstructObj.lpszValueName = (LPWSTR)ER_ID;
        memset(szValue, 0, MAX_PATH);
        mcIOstructObj.lpData = (LPBYTE)&szValue[0];
        lRes = m_cIOManager.ReadValue(mcStorageTypeObj, mcIOstructObj);
        value.clear();
        if ((LPWSTR)mcIOstructObj.lpData == nullptr)
        {
            value.assign(L"");
        }
        else
        {
            value.assign((LPWSTR)mcIOstructObj.lpData);
        }

        m_fileParams.insert({ (std::wstring) ER_ID, value });

    // Second time I am using the below statements to insert elements into the map container

        mcIOstructObj.lpszValueName = (LPWSTR)CPS;
        memset(szValue, 0, MAX_PATH);
        mcIOstructObj.lpData = (LPBYTE)&szValue[0];
        lRes = m_cIOManager.ReadValue(mcStorageTypeObj, mcIOstructObj);
        value.clear();
        if ((LPWSTR)mcIOstructObj.lpData == nullptr)
        {
            value.assign(L"");
        }
        else
        {
            value.assign((LPWSTR)mcIOstructObj.lpData);
        }
        m_fileParams.insert({ (std::wstring) CPS, value });

        return true;
    }
\$\endgroup\$
  • \$\begingroup\$ Code Review is a place to review implemented, working code. As it currently stands, your question appears to indicate that you are seeking help for not yet implemented code, which is off-topic for Code Review. \$\endgroup\$ – L. F. Aug 18 at 18:01
  • 1
    \$\begingroup\$ At least to me, it looks like this is code that is clumsy to use, but does function. Unless I'm missing something, its a fine candidate for code review. \$\endgroup\$ – Jerry Coffin Aug 18 at 22:13
  • \$\begingroup\$ Please review the code now. \$\endgroup\$ – John Paul Coder Aug 20 at 5:10
  • 1
    \$\begingroup\$ I have rolled back your changes. Once answers are made, you should not change the question in a way to invalidate any answer. \$\endgroup\$ – dfhwze Aug 20 at 5:30
1
\$\begingroup\$

There is missing code so I'm assuming that ER_ID and CPS are strings. If not, you can easily sub the data type.

Since the only difference between the blocks of code is which string (or other data type) to use (ER_ID vs. CPS), you can make a member function of EMRMgr that has the same block of code but subbing the string to use with a parameter.

Unless I've overlooked something, this should work.

bool EMRMgr::DoTheThing(std::string str)
    {
        mcIOstructObj.lpszValueName = (LPWSTR)str;
        memset(szValue, 0, MAX_PATH);
        mcIOstructObj.lpData = (LPBYTE)&szValue[0];
        lRes = m_cIOManager.ReadValue(mcStorageTypeObj, mcIOstructObj);
        value.clear();
        if ((LPWSTR)mcIOstructObj.lpData == nullptr)
        {
            value.assign(L"");
        }
        else
        {
            value.assign((LPWSTR)mcIOstructObj.lpData);
        }

        m_fileParams.insert({ (std::wstring) str, value });

    }
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.