I'm able to follow this code pretty easily. It's readable and comprehensible, which is great. In addition to the things others have said, I see a few things that could be improved.
#Naming
Generally, variable names should be related to what they represent in the real world. In the case of a
, b
, and c
, they represent the coefficients to the square, linear, and constant terms of the quadratic. You could name them as such, but since most algebra books just call them a, b, and c, I wouldn't worry too much about that. Most people reading this will know what they mean.
I would however change f
, g
, h
and k
, as those are not so common. You could name them something like factor1Coefficient
and factor1Constant
for f
and g
, respectively, and similarly for h
and k
.
I would also change the inputF
through inputK
variable names to what they represent. Something like userFactor1Coefficient
and userFactor1Constant
, etc.
The name ch
tells a reader of the code nothing. It's a char
, but we can already see that from the declaration. What is its purpose? In this case, it's to hold the user's response to whether they want to continue, so it could be named userResponse
, or keepGoing
, or something like that.
#Variable Locality
You define a
, b
, c
, level
, randMax
, and ch
at the top of main, but none of them are used until much further down in the function. Because of the function nesting you've used, these are essentially global variables (which is not a good thing). The nonZeroCoefficient()
function could, for example, change level
to something else, and it would be difficult to figure out where it changed if you were debugging a problem related to the level
variable having the wrong value.
I recommend that you do what you did with f
through k
and declare the variables where you use them. So in this case, a
, b
, and c
would be declared after f
, g
, h
, and k
.
You may object at this point and notice that level
is used in nonZeroCoefficient()
, but it can't be declared there because it needs to already be set by the time that function is called. And you are correct. Instead what you should do is pass the value into the function like this:
int nonZeroCoefficient(const int level) {
int n = (rand() % (level + 2) - level);
// ...etc.
return n;
}
Some may object to marking an int
as const
because even if you did change it in the function, the value would not be returned to the caller. However, it allows someone reading the code to understand that level
will not change in the function at all, which makes understanding the whole function easier. It's a stylistic choice that's up to your judgement. I prefer it.
#Whitespace
I recommend leaving a little more whitespace in your code. It's a bit hard for me to mentally parse:
inputF*inputK+inputG*inputH== b
I would rewrite it as:
inputF * inputK + inputG * inputH == b
As a general rule, I always put a space before and after any operator. There's a section of the book Code Complete by Steve McConnell that describes the way he formats his code and the reasoning behind each part of that decision. I found that very helpful early in my career for writing code that was easier to understand. You might find it helpful, too.
I would not, however, leave a space between the #
and the word include
when including headers. I've never seen that done anywhere. It's apparently valid, but looks very odd.
#Headers
Speaking of headers, do you need every one of those headers you include? I removed the inclusion of both <math.h>
and <ctype.h>
and everything compiled the same. It may depend on compiler settings, though. I'm not positive.