c++mfc

Hi, I created a calculator with MFC (C++), but I'm having trouble implementing parenthesis functionality


I am currently implementing a scientific calculator using MFC and C++.

I have completed most of the functionality, but I am still working on the parentheses feature.

The process of typing in the input box is completely trouble-free.

Once I type 5+5 without parentheses, I get 10.

However, when I put a number inside the parentheses, the number is treated as a zero. For example, I get (5)+5= 5.

I've spent a lot of time thinking about it, Googling, and asking GPT, but I can't get an answer.

Please let me know if I missed or doing wrong anything, thanks.

My code :

 void CMFCApplication20Dlg::OnBnClickedButtonLeftParen()
{
    CString temp;
    GetDlgItem(IDC_STATIC_VIEW)->GetWindowText(temp);
    temp += _T("("); // 왼쪽 괄호 추가
    SetDlgItemText(IDC_STATIC_VIEW, temp);
    m_LeftParenCount++;
} 
void CMFCApplication20Dlg::OnBnClickedButtonRightParen()
{
    CString temp;
    GetDlgItem(IDC_STATIC_VIEW)->GetWindowText(temp);
    if (m_LeftParenCount > m_RightParenCount)
    {
        temp += _T(")"); // 오른쪽 괄호 추가
        SetDlgItemText(IDC_STATIC_VIEW, temp);
        m_RightParenCount++;
    }
    else
    {
        AfxMessageBox(_T("Cannot add right parenthesis. Mismatched parentheses."));
    }
} 
 void CMFCApplication20Dlg::OnBnClickedButtonEquals()
{
    CString temp;
    GetDlgItem(IDC_STATIC_VIEW)->GetWindowText(temp);
    m_LeftParenCount = 0;
    m_RightParenCount = 0;
    AfterData = _ttof(temp); // 지수값(추가함)

    // CString을 double로 변환 (소수점 지원)
    double result = 0.0;
    double beforeData = _ttof(ViewStrData); // ViewStrData를 CString에서 double로 변환
    double afterData = _ttof(temp); // temp를 CString에서 double로 변환

    // % 코드임 
    int percentPos = temp.Find(_T("%"));
    if (percentPos != -1)
    {
        // `%` 기호가 있을 경우, 해당 기호를 기준으로 문자열 분리
        CString numberStr = temp.Left(percentPos); // '%' 기호 이전 부분
        double number = _ttof(numberStr); // 문자열을 double로 변환
        double result = number / 100.0;

        // 결과를 문자열로 변환하고 IDC_STATIC_VIEW에 표시
        CString resultStr;
        resultStr.Format(_T("%.2f"), result);
        SetDlgItemText(IDC_STATIC_RESULT, resultStr);
        ChangeCheck = true;
        Sign = 0;
        return;
    }

    // 계산 로직
    switch (Sign)
    {
        case 1: result = beforeData + afterData; break;
        case 2: result = beforeData - afterData; break;
        case 3: result = beforeData * afterData; break;
        case 4: 
            if (afterData != 0.0)
                result = beforeData / afterData;
            else
            {
                AfxMessageBox(_T("Zero division error!"));
                return;
            }
            break;
        case 5: 
            if (afterData != 0.0)
                result = fmod(beforeData, afterData);
            else
                AfxMessageBox(_T("Zero division error!"));
            break;
        // 추가적인 연산자 처리...
        default: AfxMessageBox(_T("Invalid operation!")); return;
    } 

    // 결과를 결과 창에 표시
    UpdateResultView(result);
    GetDlgItem(IDC_STATIC_VIEW)->SetWindowText(_T(""));
    ChangeCheck = true;
    Sign = 0;
} 

Solution

  • The core issue is this line of code:

    AfterData = _ttof(temp);
    

    The _ttof function-like macro expands to either atof or _wtof (depending on the _MBCS and _UNICODE preprocessor symbols). Either function is documented to perform the following operation:

    Each function returns the double value produced by interpreting the input characters as a number. The return value is 0.0 if the input can't be converted to a value of that type.

    When passing "5" as the input, the functions faithfully convert it to the value 5.0. Things are different for "(5)", however. While humans can easily see that the parentheses are redundant and do not affect the value, the conversion functions do not attempt to (mathematically) interpret the input. They just go ahead, read the first character ('('), and error out since that is not a valid symbol in a floating point literal.

    Instead, you'll have to write a tokenizer and parser capable of interpreting mathematical expressions in (presumably) infix notation. The Shunting Yard Algorithm is a well-known solution to the parsing problem. As noted in comments there's a YouTube video that explains the algorithm in sufficient detail: DIY Programming Language #1: The Shunting Yard Algorithm.

    The following is a rough outline of a C++ implementation of an expression parser and corresponding evaluator. It is based on the video with changes to the structure. This is the public interface:

    calc_engine.h

    #pragma once
    
    #include <deque>
    #include <string>
    
    using NumericType = double;
    
    
    namespace impl
    {
    
    struct Operator
    {
        uint8_t precedence = 0;
        uint8_t arguments = 0;
    };
    
    struct Symbol
    {
        std::wstring symbol = {};
        enum class Type : uint8_t
        {
            Unknown,
            Literal,
            Operator,
            LParen,
            RParen
        } type = Type::Unknown;
        Operator op;
    };
    
    } // namespace impl
    
    
    /// @brief Stores a mathematical expression.
    ///
    /// @remarks This type can only be instantiated by the @ref parse function.
    ///
    struct Expression final
    {
        /// @brief Evaluates the expression.
        ///
        /// @return Returns the computed result on success, throws a C++ exception
        ///         otherwise.
        ///
        [[nodiscard]] NumericType evaluate() const;
    
    private:
        Expression(std::deque<impl::Symbol>&& rpn_stk) : rpn_stk(rpn_stk) {}
        friend Expression parse(std::wstring_view input);
    
        std::deque<impl::Symbol> rpn_stk;
    };
    
    
    /// @brief Parses a mathematical expression in infix notation.
    ///
    /// @return Returns an @ref Expression on success, throws a C++ exception
    ///         otherwise.
    ///
    [[nodiscard]] Expression parse(std::wstring_view input);
    

    The important pieces are split between the top and the bottom:

    The NumericType alias declares the fundamental type used for parsing and evaluating expressions. It is introduced as a potential customization point, but doesn't do more than fail to compile if it doesn't alias double or long double at this point.

    At the other end, there's the parse() function that accepts a character string and returns an Expression that can be evaluate()-ed. Everything above and in between is just your average C++ boilerplate and the unfortunate implementation detail leakage (under the impl namespace).

    The corresponding implementation may look somewhat intimidating. The code logic closely follows that of the video, and I'll leave it to the video to explain the details.

    calc_engine.cpp

    #include "calc_engine.h"
    
    #include <cwctype>
    #include <deque>
    #include <format>
    #include <string>
    #include <unordered_map>
    #include <utility>
    #include <vector>
    
    
    namespace
    {
    
    using impl::Operator;
    using impl::Symbol;
    
    std::unordered_map<wchar_t, Operator> operators = {
        { L'/', { 4, 2 } },
        { L'*', { 3, 2 } },
        { L'+', { 2, 2 } },
        { L'-', { 1, 2 } },
    };
    
    } // namespace
    
    
    Expression parse(std::wstring_view input)
    {
        std::deque<Symbol> operator_stk;
        std::deque<Symbol> output_stk;
    
        // Track the parsing position for better error reporting
        size_t input_pos = 0;
        // "Tokenize" input
        for (auto const c : input)
        {
            if (std::iswdigit(c))
            {
                output_stk.push_back({ std::wstring(1, c), Symbol::Type::Literal });
                ++input_pos;
            }
            else if (c == L'(')
            {
                operator_stk.push_front({ std::wstring(1, c), Symbol::Type::LParen });
                ++input_pos;
            }
            else if (c == L')')
            {
                while (!operator_stk.empty() && operator_stk.front().type != Symbol::Type::LParen)
                {
                    output_stk.push_back(operator_stk.front());
                    operator_stk.pop_front();
                }
    
                if (operator_stk.empty())
                {
                    throw std::format(L"Unmatched closing parenthesis at position {}", input_pos);
                }
    
                operator_stk.pop_front();
                ++input_pos;
            }
            else if (operators.contains(c))
            {
                Operator next_op = operators[c];
                while (!operator_stk.empty() && operator_stk.front().type != Symbol::Type::LParen)
                {
                    if (operator_stk.front().type == Symbol::Type::Operator)
                    {
                        auto const& prev_op = operator_stk.front().op;
                        if (prev_op.precedence >= next_op.precedence)
                        {
                            output_stk.push_back(operator_stk.front());
                            operator_stk.pop_front();
                        }
                        else
                        {
                            break;
                        }
                    }
                }
    
                operator_stk.push_front({ std::wstring(1, c), Symbol::Type::Operator, next_op });
                ++input_pos;
            }
            else
            {
                throw std::format(L"Parsing error at position {}", input_pos);
            }
        }
    
        // Drain operator stack
        while (!operator_stk.empty())
        {
            output_stk.push_back(operator_stk.front());
            operator_stk.pop_front();
        }
    
        return { std::move(output_stk) };
    }
    
    NumericType Expression::evaluate() const
    {
        std::deque<NumericType> result_stk;
        for (auto const& item : rpn_stk)
        {
            switch (item.type)
            {
            case Symbol::Type::Literal: {
                result_stk.push_front(NumericType { std::stod(item.symbol) });
            }
            break;
    
            case Symbol::Type::Operator: {
                std::vector<NumericType> operands(item.op.arguments);
                if (result_stk.size() < operands.size())
                {
                    throw std::format(L"Too few arguments for operator {}", item.symbol);
                }
                for (uint8_t arg_count = 0; arg_count < item.op.arguments; ++arg_count)
                {
                    operands[arg_count] = result_stk[0];
                    result_stk.pop_front();
                }
    
                if (item.op.arguments != 2)
                {
                    throw std::format(L"Operator {} not implemented", item.symbol);
                }
                NumericType result = {};
                auto const op = item.symbol[0];
                if (op == L'/')
                    result = operands[1] / operands[0];
                else if (op == L'*')
                    result = operands[1] * operands[0];
                else if (op == L'+')
                    result = operands[1] + operands[0];
                else if (op == L'-')
                    result = operands[1] - operands[0];
                else
                    throw std::format(L"Unknown operator {}", op);
    
                result_stk.push_front(result);
            }
            break;
    
            default:
                if (item.symbol[0] == L'(')
                    throw std::format(L"Unmatched opening parenthesis");
                else
                    throw std::format(L"Unknown token {}", item.symbol);
            }
        }
    
        if (result_stk.size() != 1)
        {
            throw std::format(L"Unused operands");
        }
    
        return { result_stk[0] };
    }
    

    Just to verify that this is working here is a small command line program that utilizes the implementation shown above.

    Calculator.cpp

    #include "calc_engine.h"
    
    #include <iostream>
    #include <string_view>
    
    
    int wmain(int argc, wchar_t* argv[])
    {
        if (argc != 2)
        {
            std::wcout << L"Usage: " << argv[0] << L" <expression>\n";
            return -1;
        }
    
        try
        {
            auto const expr = parse(argv[1]);
            auto const result = expr.evaluate();
    
            std::wcout << L"Result: " << result << L"\n";
        }
        catch (std::wstring const& e)
        {
            std::wcout << L"Error:\n\t" << e << L"\n";
            return -1;
        }
    }
    

    After compiling and linking the application you can run it against the test expression:

    Calculator.exe "(5)+5"
    Result: 10
    

    That's reassuring. However, there are still many issues with this implementation. Notably, it makes the same "bold assumptions" as the video, i.e.,

    Other issues include:

    Exercises

    I didn't address the issues listed above under the assumption that this is a learning project. Instead, I'm leaving a list of things that can be improved alongside rationale and guidance.

    Error reporting

    The initial implementation is based on error propagation via C++ exceptions. This can work, but it places some severe restrictions on clients of the code. C++ exceptions aren't free and incredibly challenging to incorporate into existing code1. Especially for library code a less invasive strategy for error reporting is preferable, and unless you're implementing a constructor there are alternatives:

    Functionality

    A lot is missing in terms of functionality. Here is a list of issues that should be revisited:

    Robustness

    The current implementation of the evaluator merely translates operators in the expression into operations for the CPU to execute. It does nothing to (proactively) prevent operations that have no well-defined result (such as a division by zero).

    Even with just four operators (+, -, *, /) implemented, a lot of things can go wrong, given that the calculator is based on floating-point representation. Indeed, there's too much and I will encourage you to read What Every Computer Scientist Should Know About Floating-Point Arithmetic to figure out what sorts of failure you need to be prepared for. With that covered you can start working towards addressing those issues.

    Diagnostic support

    Diagnostic support is of varying quality currently. Errors that are discovered during parse()-ing are reported alongside their fault location. Issues that only surface during the evaluate() invocation (such as too many operands) cannot point to a location.

    This should be addressed. To do this, the parser needs to record and store the span in the input sequence that corresponds to any given token. The input_pos variable does this under the assumption that all tokens are exactly one character long. This needs to be changed, especially after support for (arbitrarily long) fractional was added.

    The easiest way to relay the input span relating to a given token is by adding a field to the Symbol type. This is available to both parse() and evaluate() and can be used to construct diagnostic messages.

    Customization

    The initial implementation has preliminary support for customizing the type of values. using NumericType = double; introduces a name for the underlying numeric type, but that's about all the "support" there is. It can be changed to other (integral) types, but that just leads to compiler errors for anything by double and long double.

    To improve the support two pieces of code need to be adjusted: Parsing of numeric values and the evaluation functions. The recommended approach is to get the calculator to compile with float as the NumericType to verify that things work, before moving to something more interesting like a user-defined type to represent rational numbers as numerator and denominator.

    GUI

    The question was asked in context of an MFC GUI application. The code presented can be used as-is so long as the MFC project is compiled with Unicode support (i.e., CString is an alias for CStringW). In that case, an instance can be passed to parse() and the appropriate std::wstring_view is constructed. Alternatively, { temp.GetString(), temp.GetLength() } yields a string view without calculating the length again.

    With that done, you'll want to reconsider your validation strategy. It is never a good idea to prevent a user from entering invalid data, as that is often just temporary2. A far less intrusive implementation would observe input changes (on EN_CHANGE messages) and dis-/enable the "equals" button depending on whether the input can be parsed and evaluated.


    1 This is particularly true when code executes under a foreign context (such as a message handler). When you transfer control across stack frames, all the frames in between need to be in on the joke covers the fundamental issues.

    2 WM_KILLFOCUS is the wrong time to do field validation describes usability concerns with that approach.