1. don't use
using namespace std
!
I see it often and it makes me cringe every time. A namespace is a safeguard that helps you keep names from a different library separate from your names without the need to cross check all of that other library every time you introduce a name. You may not consider it that much of an issue within the simple programs you are now writing, but it will eventually bite you when your programs get more complex! Better get used to writing the full specifiers (e. g.
std::string
instead of just
string
) now, before you get used to it so much that you forget how to do it properly.
2. If you initialize a C array with a list, don't specify the size - the size will be set automatically. Example:
string MDPOperators[] ={"*","/","^"};
instead of
string MDPOperators[5] ={"*","/","^"};
(not to mention that the count is incorrect in your code)
3. Don't write your own character classifiers (
ischar
,
isnum
). They already exist in the standard library, and are much less error prone and more stable than you will be able to write them. See
http://www.cplusplus.com/reference/cctype/isalpha/[
^] and
http://www.cplusplus.com/reference/cctype/isdigit/[
^]
4. Don't use numbers to delimit your loops, the numbers will eventually be wrong, the reader may not realize where you got the number from and how to correct it if the code doesn't work! Instead either use constants that are defined at a proper place in your code, or derive the number of loop iterations from the container you work on.
Example, in function keycheck(), change
for(int i=0;i<2;i++)
to
int nkeys = sizeof(keyword) / sizeof(keyword[0]);
for (int i = 0; i < nkeys; ++i)
5. Avoid deep nesting levels. Once you have four or more levels of nesting due to excessive use of
if
and loops, redesign the function! One way to do it is using
switch()
/
case
instead of a series of
if
cases. Another is to extract the inner part of your nested code into a separate function. Either way, it is much easier to understand what a function does (or doesn't, against expectation), when you keep the nesting level low.
6. Write test code. It is a lot easier to pinpoint a problem when you know whether or not each individual function works as expected. For example, you can write a simple test function that performs a series of calls to keycheck() with specific strings and verifies that the result is correct.
Example:
bool test_keycheck() {
int success = 0;
int tests = 0;
if (!keycheck("123")) {
++success;
}
++tests;
if (keycheck("string")) {
++success;
}
++tests;
std::cout << "keycheck() tests performed, " << success << " out of " << tests
<< " tests performed successfully." << std::endl;
return success==tests;
}
7. Learn to use a debugger - see solution 1
(and I only added this as a last point because it has already been covered before,
not because I consider it the least important!)