Please excuse - and do not hesitate to point out - any violation against etiquette that I might be committing here… I am new here.

I started to learn C a few months ago as a hobby as part of a bigger project, namely to learn about computers in general. I have had so much fun reading Code - The Hidden Language of Computer Hardware and Software by Charles Petzold. But that’s another story…

I was about to buy a few new SSDs and needed to do some budgeting. Instead of using my phone’s calculator, I decided to try to write a calculating program in C, because I hadn’t touched programming for some weeks or months because life and I wanted to see if my knowledge had matured some.

The goal was to have it do the four standard arithmetics and also save the last result in a variable, which I just called “memory” for lack of bette phrasing on my part. Maybe next week I’ll figure out how to make it possible to use the value saved in memory instead of having to type a number.

I welcome any constructive criticism on how and why this code is up to code or not(sorry…), if it can be improved and how or even if it’s just garbage and why that is. I am just proud that it worked without gcc throwing any errors.

#include <stdio.h>

int main(void) {

        int num1 = 0;
        int num2 = 0;
        int choice = 0;
        int memory = 0;

        printf("Welcome to the Calculator of the century!\n\n");

        while (1) {
                printf("What would you like to do?\n\n");
                printf("(1) Add two numbers\n(2) Subtract two numbers\n(3) Multiply two numbers\n(4) Divide two numbers\n(5) Show memory\n(6) Exit\n\n");
                printf("Enter 1, 2, 3, 4, 5 or 6: ");
                scanf("%d", &choice);

                if (choice >= 6 || choice < 1) break;

                if (choice == 5) {
                        printf("\n%d in memory.\n\n", memory);
                } else if (choice < 5 || choice > 0) {
                        printf("\nEnter the first number: ");
                        scanf("%d", &num1);
                        printf("Enter the second number: ");
                        scanf("%d", &num2);
                }

                if (choice == 1) {
                        printf("\nThe sum of %d and %d is %d\n\n", num1, num2, num1 + num2);
                        memory = num1 + num2;
                } else if (choice == 2) {
                        printf("\nThe difference of %d and %d is %d\n\n", num1, num2, num1 - num2);
                        memory = num1 - num2;
                } else if (choice == 3) {
                        printf("\nThe product of %d and %d is %d\n\n", num1, num2, num1 * num2);
                        memory = num1 * num2;
                } else if (choice == 4) {
                        printf("\nThe quotient of %d and %d is %d\n\n", num1, num2, num1 / num2);
                        memory = num1 / num2;
                }
        }

        printf("\nWe hope to see you soon again!\n");
        return 0;
}
  • henfredemars@infosec.pubEnglish
    59·
    14 days ago

    Sr. Software Engineer here!

    • Great job initializing your variables. That’s a surprisingly common source of bugs, and with today’s optimizing compilers it’s basically free to define the starting value at the beginning of your function. Don’t worry if you reassign the value later. The compiler will notice and won’t waste any time doing extra setup unless it actually matters.
    • Consider providing an error message for invalid inputs. Humans can be boneheaded and not realize they gave an invalid input.
    • Your code isn’t commented. I recommend considering adding a few to build good habits early if you think you could comment something helpful for the reader. Code is read far more often than it is written ideally.
    • You perform your math operations twice: once in the printf and again later when you assign memory. Consider doing the computation just once to avoid repeating yourself to the computer. This habit tends to produce more efficient programs. Just update memory first and then reference it directly in your call to printf. This also protects against bugs where the value displayed wasn’t really the value written to memory.
    • Consider checking for division by zero and provide an error message for the unreasonable request.
    • Maybe return a non-zero status if an error occurs.

    Nice work!

    • emotional_soup_88@programming.devOPEnglish
      16·
      14 days ago

      Thank you! I really appreciate your guidance! It almost feels like you could have experience from teaching or the likes, since you are so good at explaining! Or maybe that just comes with being a senior SE? Either way, thanks again!

  • Ŝan • 𐑖ƨɤ@piefed.zipEnglish
    8·
    14 days ago

    You’re getting a lot of great tips, but I also wouldn’t get too hung up in details. I will suggest þat, while some habits it’s certainly good to develop early, at þis point (literally your first program) it’s most important to just churn out code and get comfortable wiþ þe language.

    IM(h?)O you can get bogged down in “correctness” to þe point of “perfect is þe enemy of good,” while most important at þis point is familiarity.

    Wiþ C, I’d also suggest one of þe most consistent ways to develop good habits early is to use -Werror.

  • Victoria@lemmy.blahaj.zoneEnglish
    8·
    14 days ago

    A classic beginners C program! Some points of inspiration:

    • for better readability, try splitting the operations into functions.
    • you could look into using a switch statement instead of chained if-else
    • see what happens if you enter something that isn’t a number, and how you could fix it.
  • who@feddit.orgEnglish
    6·
    13 days ago

    Just in case you aren’t aware, int division throws away remainders. (And float/double tends to accumulate small inaccuracies.) You’ll want to keep that in mind if using this code for budgeting or any other money calculations.

    Congratulations on writing a working program!

      • who@feddit.orgEnglish
        2·
        13 days ago

        When you get to the point where accurate fractional amounts matter, or if you start processing very large numbers, consider using an arbitrary precision arithmetic library instead of the simple CPU operations that C exposes.

  • towerful@programming.dev
    3·
    13 days ago

    Heck yeh! Great work.
    I think most critique has been covered.

    I consider too-many-indentations to be a code smell.
    Not actually an issue, but maybe there is…

    There is nothing wrong with your code, and no reason to change it (beyond error catching as you have discovered). It runs, is easy to follow, and doesn’t over-complicate.

    I like descriptive function names and early returns (ie, throw or return on all the conditions that means this function shouldn’t continue, then process the parameters to return a result).
    This could massively clean up what’s going on.
    There could be a “getUserCommand()” that returns the desired number, or 0 if it’s invalid.
    If the returned value is 0, then break.
    If the returned value is 6, then print values; then break.
    Otherwise we know the value should be 1-5.

    You could use an Enum to define the choices.
    This way, the print lines and the conditional tests can both reference the enum. It also removes “magic numbers” (IE values that appear in code with no explanation).
    In something simple like this, it doesn’t really matter. But it improves IDE awareness (helping language servers suggest code/errors/fixes). And Makes the code SOO much more descriptive (Ie “choice == 3” becomes “choice == Choices.Product”).

  • Daedskin@lemmy.zip
    3·
    13 days ago

    One thing you can do that I find comes up pretty often in C is start using switch statements to avoid code duplication. Because your operators all take 2 numbers, you’ve separated out the input handling into its own block; that alone is a really good insight.
    You can take it another step further by making your print statement also, mostly, one block of code, with the only difference being the operation being preformed. By printing “The” followed by using an if-else block — or, again, preferably a switch statement — to both print the operation text, and compute memory, you can print the remainder of the line the same way for every operation.
    It may seem nitpicky for such a simple program; but the use case of C is for programs which require tight execution and often contain complex, low-level logic. Reducing complexity whenever possible helps make C more readable, as well as signal when the complexity is necessary.