LinuxQuestions.org - Looking for critique. Teaching myself C.

Page 1 of 2

Show 50 post(s) from this thread on one page

- Programming (https://www.linuxquestions.org/questions/programming-9/)

- - Looking for critique. Teaching myself C. (https://www.linuxquestions.org/questions/programming-9/looking-for-critique-teaching-myself-c-4175720597/)

jmgibson1981

01-06-2023 05:21 PM

Looking for critique. Teaching myself C.

https://gitlab.com/jmgibson1981/mycprogress

Thus far I've only made a very basic calculator. It works. I'm sure it won't be my last. I'm hoping it's ok to ask for constructive criticism. Thank you for your help and interest.

EdGr	01-06-2023 09:33 PM

That link leads to an endless spinner.
Ed

ntubski

01-06-2023 10:36 PM

It would be good to handle unrecognized operators with some sort of error indication.

You don't seem to be returning a value from math_line or main? I guess the compiler should be warning you about that.

Quote:

Originally Posted by EdGr (Post 6402715)

That link leads to an endless spinner.

You have JavaScript disabled? Try https://gitlab.com/jmgibson1981/mycp...culator/calc.c

EdGr	01-07-2023 10:26 AM

The direct link works. Otherwise, there is some problem with SeaMonkey and GitLab.

jmgibson1981 - Compile the program with -Wall. Gcc will complain about the lack of return values from functions.
Ed

pan64

01-07-2023 10:45 AM

avoid code duplication, so

Code:

        switch (b) {

                case '+':

                        t = a + c; break;

                case '-':

                        t = a - c; break;

                case '*':

                        t = a * c; break;

                case '/':

                        t = a / c; break;

        }

        math_ret = print_line(t);

Also use a default for that switch (in case something different was entered).
multiplication is ok, just the shell itself interprets * before passing to your app. As you wrote you can avoid it using '*'
Check division by zero.

rtmistler

01-07-2023 11:30 AM

Use real, meaningful variable names.

Do not combine multiple lines of code in one physical line.

sundialsvcs

01-07-2023 12:28 PM

"The very first computer program that I ever wrote was eight lines long, took me six months to write, and had a bug in it." :D

#define PREACHER_MODE

When you are writing any "algorithm," whether it be large or small, you should also be thinking about test cases. You should be thinking, simultaneously, about cases which demonstrate correct behavior, and(!) about cases which represent errors. "A calculator" should respond "intelligently" to any(!) string of characters that is given to it. It should never "crash." No matter what.

In many languages – notably, Perl – packages are accompanied by sometimes-many-dozens of "automatic tests" which are applied before the package will consent to be installed. These tests help to verify that the package will, in fact, perform as intended on your computer. The formulation of these tests is somewhat of an art, but I have found that it is a very important discipline to develop. These disciplines will help you to say with confidence, not only that "your algorithm works," but that "it still works." (I promise you that you will be surprised when an "unrelated" change suddenly causes a test to fail. But, you will be very glad that the computer just caught it.)

#undef PREACHER_MODE

Welcome aboard. ("Welcome to my nightmare ...") Never be too hard on yourself. It can be a crazy way to make a living. :)

jmgibson1981

01-07-2023 06:54 PM

Ok I'm still not sure about those test cases but I did make suggested changes. I also added the ability for it to detect remainders on a division solution. changed up the commenting a bit as well.

As far as the returns. I have been compiling with gcc and haven't seen any errors however i did add some to a few places. Trial and error a bit but it seems to be working just fine now. I await further guidance is you have any.

https://gitlab.com/jmgibson1981/mycprogress

ntubski

01-07-2023 10:12 PM

Quote:

Originally Posted by jmgibson1981 (Post 6402949)

As far as the returns. I have been compiling with gcc and haven't seen any errors

Try adding -Wall and -Wextra flags:

Code:

cc -Wall -Wextra    lq-calc.c  -o lq-calc

lq-calc.c: In function 'main':

lq-calc.c:27:18: warning: variable 'ret' set but not used [-Wunused-but-set-variable]

  27 |  int num1, num2, ret;

      |                  ^~~

lq-calc.c: In function 'math_func':

lq-calc.c:49:6: warning: variable 'math_ret' set but not used [-Wunused-but-set-variable]

  49 |  int math_ret;

      |      ^~~~~~~~

Also, if you add -O1 optimization, the compiler will notice another problem:

Code:

lq-calc.c:73:13: warning: 'solution' may be used uninitialized in this function [-Wmaybe-uninitialized]

  73 |  math_ret = print_func(solution);

      |            ^~~~~~~~~~~~~~~~~~~~

jmgibson1981

01-07-2023 10:45 PM

Was able to remove the math_ret and ret

If I comment out solution int then it fails to compile saying that it isn't aren't initialized.

Code:

$ gcc -Wall -O1 -o calc.e calc.c

calc.c: In function ‘math_func’:

calc.c:82:2: warning: ‘solution’ may be used uninitialized in this function [-Wmaybe-uninitialized]

  82 |  printf("answer is %i\n", solution);

      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I've got lots to learn. thank you again for the assistance and information.

As it sits now.

https://gitlab.com/jmgibson1981/mycprogress

pan64

01-08-2023 03:13 AM

this is much better, although other operations (illegal chars) are still not checked.
For example what will print: calc.e 1 q 5 ?
also the main function should return 0 only if everything was ok, otherwise something else (1).

ntubski

01-08-2023 09:32 AM

Quote:

Originally Posted by jmgibson1981 (Post 6402969)

If I comment out solution int then it fails to compile saying that it isn't aren't initialized.

Quote:

Originally Posted by pan64 (Post 6402996)

this is much better, although other operations (illegal chars) are still not checked.
For example what will print: calc.e 1 q 5 ?

Yes, these two things are related. Specifically, the problem with solution not being initialized will be triggered when the user passes an illegal operator (and none of the switch cases are chosen).

jmgibson1981

01-08-2023 01:29 PM

Very good. Did those. Got an empty response on

Code:

gcc -Wall -o calc calc.c

Thank you. Have learned a lot from this one alone.

business_kid

01-08-2023 02:11 PM

Reading this thread makes me very glad I never pursued software. Hardware was bad enough.

ntubski

01-08-2023 03:52 PM

Quote:

Originally Posted by jmgibson1981 (Post 6403115)

Very good. Did those. Got an empty response on

Code:

gcc -Wall -o calc calc.c

Hmm, I think that's only because gcc doesn't do the analysis needed to make that warning unless you ask for optimization; I still get the warning if I include -O1. The problem is that gcc doesn't analyse across functions, so it doesn't see what you handle the wrong operator in the upper level. In this case I think it would be better to check for a wrong operator in math_func, because you currently have two places where the valid operators are listed which means you can have bugs if they don't match.

All times are GMT -5. The time now is 07:02 PM.

Page 1 of 2

Show 50 post(s) from this thread on one page