Regarding private help with messy code...

[nixer526]

Hi there people. I'm very new to programming and have just finished my first simple scientific calculator program.

The problem is that my program uses too many loops. From what I know, too many loops means that it'll take a lot of time for the program to run through everything, which is a bad thing...

I know that my code (and the way I program) needs a lot of improvement and so I'm wondering if someone can help me with it.

Right now, I don't know if posting my code up on the forum is the best way to do it, or if we should discuss it privately.

If you are willing to help me to improve my code (and my programming skills + knowledge), please do let me know if you prefer do it on the forum or on skype (or msn).

Thank you in advance.

[Moschops]

From what I know, too many loops means that it'll take a lot of time for the program to run through everything, which is a bad thing...

There is nothing inherently time-consuming about a loop. It's a piece of code. Yes, writing out the same code twice will be faster than looping over one piece of code twice, but as a general rule of thumb, when you want to make your code execute faster, unrolling loops (whilst it is on the list of things to think about) should not be at the top of the list.

The most effective speed gains I've seen (apart from porting code from a battered Vic 20 to a 21st century supercomputer) generally involve thinking about the algorithms and finding better ones.

[nixer526]

Thank you for your reply. Yes, I do realize that having a good algorithm do make a lot of difference. I'll try my best to learn more about algorithms along the way.

However, its hard for me to learn from just reading things, which is why I've set a simple exercise for my self and hope that some experience programmer would be willing to guilde me.

My program is working but I might have a lot of coding mistake in it, which is why I would rather wait for someone who is willing to help me before posting up my code.

[Chervil]

Well, maybe just post a small part of your code, perhaps just one typical function. It's likely that any issues in one part of the code will be echoed throughout, so you don't need to post everything to get some help.

[TheIdeasMan]

@nixer526

Don't be afraid of posting your code on the forum. We will always try to give constructive criticism - which is a good thing for you because you can learn from it. If anyone is being unfair to you, then we will say so. The worst thing you might hear is someone gently suggesting to start again (If the code is really bad).

There are lots of people who are willing to help - that is what the forum is all about.

To keep everyone happy, always use code tags - the <> formatting button on the right, post your compiler output in full if there is any, or any runtime messages in full if there are any of those. The best thing is to take on board what advice is being given, help your self by looking up documentation / reference (there is lots on this page at the top left). If you can demonstrate that you try to figure things out yourself, then people will much more likely to to spend some more time helping you out.

Hope all goes well, good luck. Cheers

[nixer526]

@Chervil
Thanks for all the advice. I'll be posting my code at the very top.
It is very ugly and messy, but I've tried to comment as much as I can.

@TheIdeasMan
Thank you very much for you kind reply. I do take criticism well because I do know that it will help me to become a better programmer.

As mention up above, my code is kinda messy, and I'm working hard to improve it. If I did do anything wrong, please do let me know.

Thanks in advance.

[nixer526]

edit: code being removed as no one seems to be replying...Thanks to those who have been willing to spend some time to give me some feedback on improving my code.

[nixer526]

edit: code being removed as no one seems to be replying...Thanks to those who have been willing to spend some time to give me some feedback on improving my code.

[nixer526]

The following is the outcome of my code.

---------------------------
* Welcome to my Calculator*
*  To leave type  "exit" *
*     Type when ready     *
---------------------------
1+1
add func a: 1 b: 1
2
Answer: 2
---------------------------
* Welcome to my Calculator*
*  To leave type  "exit" *
*     Type when ready     *
---------------------------
1+(2*3)-4
mult func a: 2 b: 3
6
add func a: 1 b: 6
7-4
subt func a: 7 b: 4
3
Answer: 3
---------------------------
* Welcome to my Calculator*
*  To leave type  "exit" *
*     Type when ready     *
---------------------------
-1*(-1-5)
subt func a: -1 b: 5
-6
mult func a: -1 b: -6
6
Answer: 6
---------------------------
* Welcome to my Calculator*
*  To leave type  "exit" *
*     Type when ready     *
---------------------------
exit

So far, I've track down 1 problem with my code, which deals with 'e'.
If there's any other problem, please do let me know.

Thanks in advance.

[TheIdeasMan]

Well that is quite an awesome program, parsing expressions with parentheses is quite tough - WELL DONE!!

Just a few things that caught my eye:

Do not use goto - forget it even exists. Your usage was to exit the program. If you want to print messages and then exit, put into a function. Make use of the exit function to leave the program.

Avoid using infinite loops. There are valid places to use them, but this isn't one of them. Here is a better way to quit out of something:

bool Quit = false;
while (!Quit) {

//your code here
//user wants to quit
Quit = true;
}

I think you need to make more use of functions - your runmath function is 160 lines long. Candidates for functions are the body of loops, and long if or else if clauses. Also look out for repetitive code such as this :

if (a[counterB - 1] == '*' ||
                    a[counterB - 1] == '/' ||
                    a[counterB - 1] == '+' ||
                    a[counterB - 1] == '-') {   //check to see if its a negative number
                    counterB -= 1;
                }

This could be a IsOperator function with a switch inside ( easily extendable).

With the bracket counting, you could increment for left brackets and decrement for right brackets. Then if bracketcount is not zero there is an error.

Does the divide function check for division by zero?

Any way the whole thing is a really good effort - you are being too modest calling your self a beginner.

Hope all goes well, I won't be able to reply for awhile - off to bed, it's 2:20am here !!

[nixer526]

@TheIdeasMan

Do not use goto - forget it even exists. Your usage was to exit the program. If you want to print messages and then exit, put into a function.

I will keep that in mind. I don't know why I've never thought of that...

Avoid using infinite loops. There are valid places to use them, but this isn't one of them.

I have never really thought about this before...I do know the danger of infinite loops, so I'll be changing to what you have suggest.

I think you need to make more use of functions - your runmath function is 160 lines long. Candidates for functions are the body of loops, and long if or else if clauses. Also look out for repetitive code

I have been trying very hard to put them into functions, but I'm not really good with it right now...When the code was first completed, everything was being stick together... I guess I'll work harder on my logic part on breaking things down. I'll keep an eye out for repeaters.

IsOperator

Thanks for the suggestion, I'll work on it.

With the bracket counting, you could increment for left brackets and decrement for right brackets. Then if bracketcount is not zero there is an error.

I did coded this way at first, but then I realized that I needed a counter, which is why I've done it this way...I guess my logic is a bit limited, which is why i've done it the "bad" way...

Does the divide function check for division by zero?

I did think about this, but then, when I was running the program, it seems to print out "inf" which I think it means infinite...so I didn't really look into it. I think I was being lazy....

---------------------------
* Welcome to my Calculator*
*  To leave type  "exit" *
*     Type when ready     *
---------------------------
1/0
divi func a: 1 b: 0
inf
Answer: inf

Thank you very much for your reply. It does open up some new doors to me and I'll be sure to look through those points.
Thank you again.

Edit--------------------------------------------
Just checked my code, it wasn't working with equation like this "1+(2+(3+4))". I have fixed it and updated my code above.

I won't be updating suggestion made by testers yet. I want to have different viewers giving different suggestion from the same piece of code.

However, do note that all suggestion are being taken seriously and I'll be doing changes at my end.

Once again, thank you all for reading this and helping me out. I very much appreciate it.

[nixer526]

A small bump as I would like to have some more reviews from people.

One thing that I've just learned is that not having a loop inside a loop, however with my code, there are lots of loops which are inside of another loops...

Can anyone suggest any way to improve it!?

Thanks in advance

[TheIdeasMan]

Maybe you should try a new thread titled "Parsing mathematical expressions".

Point out that it is related to this thread & provide a link to it, so people know what's going on.

There are other ways to do this, but I don't have any experience with them. I am sure there are plenty of more experienced people who do know how to do this.

Reverse Polish Notation (RPN) calculators are easy to do with a stack, and don't need parentheses.

Hope this helps.