calculator icon indicating copy to clipboard operation
calculator copied to clipboard

Handle "Subtract" ( "-" ) Operation as "Toggle Sign" ( "+/-" ) in Special Cases to fix #1541 and #1311

Open drxddy opened this issue 1 year ago • 5 comments

Fixes #1541

Fixes #1311

This Pull request addresses these following issues:

  1. #1541

  2. #1311

by Handling "Subtract" ("-") Binary operator as "Toggle Sign" ("+/-") in Special cases.

Description of the changes:

  • To solve #1311, I modified IsGuiSettingOpCode method inside src/CalcManager/CEngine/CalcUtils.cpp to save IDC_EXP in CCalcEngine::m_nLastCom to check if next input is IDC_SUB (Subtract Operation) and also m_nLastCom is IDC_EXP so we can redirect this operation into IDC_SIGN (+/-) operation.
  • To Solve #1541, I created a boolean flag CCalcEngine:m_bFlagSign to store, subtracting with zero as a "Toggle Sign" Operation (IDC_SIGN), so that the following digit inputs (the second operand) will be of negative sign, following is the piece of code from the pull request where the same is done.
        /*
            Addressing issues:
                #1541 https://github.com/microsoft/calculator/issues/1541
                #1311 https://github.com/microsoft/calculator/issues/1311
            Solution:
                Consider the previous '-' Binary Op as a '+/-' sign value if
                the first opnd is 0.
        */
        if (m_bFlagSign)
        {
            wParam = IDC_SIGN;
            m_bFlagSign = false;
        }

How changes were validated:

  • I have used manual testing in reference to the images provided and descriptions of the issues #1541 and #1311 and following is the output

Output for #1541

https://github.com/microsoft/calculator/assets/59935432/e347b974-bbd1-48ea-afd7-8054c35a5ec9

Output for #1311

https://github.com/microsoft/calculator/assets/59935432/76e64298-d55b-433f-a4cd-9fb68e4b1d6e

drxddy avatar Aug 06 '23 14:08 drxddy

@microsoft-github-policy-service agree

drxddy avatar Aug 06 '23 15:08 drxddy

Hi @grochocki, @hanzhang54

can you please review these changes.

drxddy avatar Aug 06 '23 18:08 drxddy

/azp run

tian-lt avatar Aug 21 '23 06:08 tian-lt

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Aug 21 '23 06:08 azure-pipelines[bot]

I have not changed the UnitTests module to expect the new changes, because the Unit Test module was taking long to run on my PC, wasn't clear if they passed or failed, getting a black screen with two numbers, and I could not find a relevant test function that's testing these cases:

  1. 2.6e-2 = 0.026
  2. -128 input

I think that's why UnitTests x64 has failed.

drxddy avatar Aug 21 '23 06:08 drxddy