0

I am working on a small GUI calculator project and I need to append something to the contents of an edit control. The function I am currently using is:

int CharControl = 256;

void AddToEditMainText(char WhatToAdd[CharControl]) {
    char CurrText[CharControl] = "";
    GetDlgItemText(MainWindow, EDIT_MAIN, CurrText, CharControl);
    char NewText[CharControl] = "";
    malloc(strlen(NewText) + strlen(WhatToAdd) + 1);
    strcpy_s(NewText, CurrText);
    strcat_s(NewText, WhatToAdd);
    SendMessage(EditMain, WM_SETTEXT, NULL, LPARAM((LPCSTR)&NewText));
}

I'm relatively new to C++, so this is my first GUI project. Any help is greatly appreciated. Thanks.

Forrest4096
  • 159
  • 1
  • 8
  • `WM_SETTEXT` will replace the entire contents. To append, you can position the cursor at the end with `EM_SETSEL` and then use `EM_REPLACESEL` to append. – Jonathan Potter Aug 28 '14 at 01:09
  • Don't use Code::Blocks - it's notorious for using ill-fated defaults. Windows NT has been UTF-16 since its inception. Don't use `char`s - `wchar_t` is where it's at. – IInspectable Aug 28 '14 at 16:25
  • I'm not using code::blocks, I'm using visual studio 2013. – Forrest4096 Aug 29 '14 at 08:48

2 Answers2

2

You are not assigning the result of malloc() to anything, so you are leaking memory. You should be doing something more like this instead:

void AddToEditMainText(char *WhatToAdd)
{
    int iLen = GetWindowTextLen(EditMain) + strlen(WhatToAdd) + 1;
    char *NewText = malloc(iLen);
    if (NewText)
    {
        GetWindowText(EditMain, NewText, iLen);
        strcat_s(NewText, iLen, WhatToAdd);
        SetWindowText(EditMain, NewText);
        free(NewText);
    }
}

I would suggest a different approach that does not require you to retrieve the current text before appending to it:

void AddToEditMainText(char *WhatToAdd)
{
    DWORD dwStart, dwEnd;
    SendMessage(EditMain, EM_GETSEL, WPARAM(&dwStart), LPARAM(&dwEnd));
    int iLen = GetWindowTextLength(EditMain);
    SendMessage(EditMain, EM_SETSEL, iLen, iLen);
    SendMessage(EditMain, EM_REPLACESEL, TRUE, LPARAM(WhatToAdd));
    SendMessage(EditMain, EM_SETSEL, WPARAM(dwStart), LPARAM(dwEnd));
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Similarly, at a later stage, I will have to code some sort of backspace method. I found using your method of EM_SETSEL and EM_REPLACESEL to be useful, so is there a way to make a backspace function using that? – Forrest4096 Aug 28 '14 at 02:03
  • Simply use `EM_SETSEL` to select the last character and then `EM_REPLACESEL` it with a blank string. – Remy Lebeau Aug 28 '14 at 02:16
1

Let's change a few things:

void AddToEditMainText(char *WhatToAdd)

In the context of function parameters, a character array and a pointer is the same thing. Even if the passed parameter is a char array of a given length, it doesn't matter, so let's not confuse things.

But we can do even better. This question is tagged C++, not C. In C++, we'd like to think we've progressed a bit above low-level byte flinging. When we're talking strings, we're now talking std::string:

void AddToEditMainText(const std::string &WhatToAdd)

I'm not very familiar with MS-Windows API, but it seems obvious that "GetDlgItemText" is a C API function that pulls text from an existing dialog control. Not sure if you're guaranteed that the returned string will be less, or will be truncated to a 256 byte buffer. That's actually an important point, but, for an introductory environment for C++, that's not important.

The char buffer used by GetDlgItemText() is so ... C-ish. A char buffer is such a quaint concept in C++. After getDlgItemText returns, let's not waste any time, and get it into a nice, pretty, std::string:

std::string CurrTextStr=CurrText;

Now, again, in C++ we don't have to bother with petty details like allocating memory, and making sure to free it after use. std::string will do it for us:

std::string combinedString=CurrTextStr + WhatToAdd;

Wasn't this easy? No need to calculate sizes of various buffers. That's so ...last century. We're in the modern age, where the "+" operator will do it for us. And, finally, since SendMessage() is another quaint C library API, and we have to talk down to its level, I suppose...

SendMessage(EditMain, WM_SETTEXT, NULL, LPARAM((LPCSTR)combinedString.c_str()));

Oh, and don't forget to stick a "#include <string>" somewhere at the top, to pull in the definition of std::string.

Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
  • That cast to `LPCSTR` just raises my suspicion until I realize it's just casting to the same type as the operand. – chris Aug 28 '14 at 01:25
  • Using references to C++ objects in an interface is not something I would call *"doing even better"*. Doing so severely limits the applicability. You just went from *"can be invoked from just about any programming language in the universe"* to *"only use from C++, using this particular compiler, and this particular CRT, and provide **exactly** these compiler options"*. Just leave it at `const char*` and move on. – IInspectable Aug 28 '14 at 16:23