[PATCH]calc:Add a simple implementation.

Alex Henrie alexhenrie24 at gmail.com
Thu Mar 22 21:50:34 CDT 2018


This looks like a good start. Some specific suggestions:

+char string[100] = "";

"string" is not a very descriptive name. Something like "input_value" would
be better.

+typedef struct BUTTON_SETTING
+{
+    char content[3];
+    int x;
+    int y;
+    int nWidth;
+    int nHeight;
+    int hMenu;
+} Buttons;

+    Buttons Button[17];

I think it would be better to have a static table of button definitions.
Something like:

static const struct
{
     char content[3];
     HMENU hMenu;
}
buttons[] =
{
     {"7", ID_CALC_7},
     {"8", ID_CALC_8},
     {"9", ID_CALC_9),
}

Create single variables in WinMain for x, y, width, height, and hMenu.

+    hwnd = CreateWindowExA(0, "Calculation", "Calculation",

The window title should be "Calculator".

+            case 12: {strcpy(Button[i].content, "DEL"); Button[i].hMenu =
ID_CALC_C;} break;

The "DEL" button should be labeled "C" (for "Clear"). I don't think I've
ever seen a 4-function calculator with this button labeled anything other
than "C".

+    for(int i = 0; i < 17; i++)

C89 forbids declaring a variable in a for statement. Declare int i at the
top of the function instead. Also, once you have the static list, say
sizeof(buttons)/sizeof(buttons[0]) instead of hardcoding the value 17.

+        /* set special buttons such as "=" "0" */

The comment is mistaken: It doesn't look like there's anything special
about = and 0.


General comments:

- The subject line is missing a couple of spaces. It should be something
like "[PATCH] calc: Add a simple implementation."

- I don't think we want an algebraic calculator: The user shouldn't be able
to input 1+2/3, for example. The current operator (+, -, *, /) should be
shown above the input value.

- After clicking =, you should be able to click another operator, input
another number, and see the result of that operator with the last number
and the new number. For example, if I enter 1+9 and click = so that the
calculator says 10, clicking / 2 = should cause the number 5 to appear.

- The calculator crashes if too many digits are entered.

- Running `./configure --enable-win64 && make` shows a compiler warning:

In file included from ../../include/windows.h:40:0,
                  from main.c:22:
main.c: In function ‘WinMain’:
main.c:274:39: warning: cast to pointer from integer of different size
[-Wint-to-pointer-cast]
                                        (HMENU)Button[i].hMenu,
                                        ^
../../include/winuser.h:3495:184: note: in definition of macro
‘CreateWindowA’
  #define
CreateWindowA(className,titleName,style,x,y,width,height,parent,menu,instance,param)
CreateWindowExA(0,className,titleName,style,x,y,width,height,parent,menu,instance,param)


                                 ^~~~

- It's easier to review patches sent with `git send-email`.


Finally, if you're planning to propose implementing calc.exe as your Summer
of Code project, you might want to find some easy bug to fix for your first
patch and save calc.exe for later. Your first patch (which qualifies you to
apply for Summer of Code) can be to any part of Wine.

-Alex



More information about the wine-devel mailing list