3
\$\begingroup\$

Here is the code I wrote for my mother's dental office. It fulfills these tasks:

  • determines whether a tooth is anterior or posterior
  • determines if the patient needs a filling
  • sets up a later appointment

My issue is with cleaning the code to increase readability, because I think there is a better way to write it. I also need help outputting the answers to a .txt file for later reference, but I'm not sure if that will screw up the code.

I am newbie, so tips are very much appreciated. I have a lot of bad habits when it comes to programming.

// fillings program
#include <iostream>
using namespace std;

int main()
{
    bool post,              // posterior: ?
         ante;              // anterior: ?
    int fillings;           // fillings: Y/N
    int tooth_number;       // tooth_number: 1-32
    int surface_num;        // surface_num should be 1-5
    int amal_or_comp;       // amalgum or composite filling
    int cav_deep;           // input for if cavity is deep

    cout << "Is the cavity deep?: 1.Yes 2.No" << endl;
    cin  >> cav_deep;
    if (cav_deep==1)
        cout << "Temporary filling needed. Patient needs to come back for permanent      fill at later date." << endl;
        else if (cav_deep==2)
    {
        cout << "Fillings Needed?: 1.Yes 2.No" << endl;
        cin  >> fillings;
        if (fillings==2)
        {
            cout << "Proceed to 'crowns'" << endl;
        }
        if (fillings==1)
        {
            cout << "Tooth #: " << endl;
            cin >> tooth_number;
            cout << "Tooth # entered is: " << tooth_number << endl;
            // if tooth is posterior
            if (tooth_number>=1&&tooth_number<=5)
            {
                cout << "Posterior Tooth" << endl;
                cout << "1. Amalgam or 2. Composite?" << endl;
                cin  >> amal_or_comp;
                if(amal_or_comp==1)
                    cout << "Amalgam selected." << endl;
                if(amal_or_comp==2)
                    cout << "Composite selected." << endl;
                cout << "Surface #: ";
                cin  >> surface_num;
                cout << "Surface # entered: " << surface_num;
            }
            if (tooth_number>=12&&tooth_number<=16)
            {
                cout << "Posterior Tooth" << endl;
                cout << "1. Amalgam or 2. Composite?" << endl;
                cin  >> amal_or_comp;
                if(amal_or_comp==1)
                    cout << "Amalgam selected." << endl;
                if(amal_or_comp==2)
                    cout << "Composite selected." << endl;
                cout << "Surface #: ";
                cin  >> surface_num;
                cout << "Surface # entered: " << surface_num;
            }
            if (tooth_number>=17&&tooth_number<=21)
            {
                cout << "Posterior Tooth" << endl;
                cout << "1. Amalgam or 2. Composite?" << endl;
                cin  >> amal_or_comp;
                if(amal_or_comp==1)
                    cout << "Amalgam selected." << endl;
                if(amal_or_comp==2)
                    cout << "Composite selected." << endl;
                cout << "Surface #: ";
                cin  >> surface_num;
                cout << "Surface # entered: " << surface_num;
            }
            if (tooth_number>=28&&tooth_number<=32)
            {
                cout << "Posterior Tooth" << endl;
                cout << "1. Amalgam or 2. Composite?" << endl;
                cin  >> amal_or_comp;
                if(amal_or_comp==1)
                    cout << "Amalgam selected." << endl;
                if(amal_or_comp==2)
                    cout << "Composite selected." << endl;
                cout << "Surface #: ";
                cin  >> surface_num;
                cout << "Surface # entered: " << surface_num;
            }
            // if tooth is anterior
            if (tooth_number>=6&&tooth_number<=11)
            {
                cout << "Anterior Tooth" << endl;
                cout << "Composite" << endl;
                cout << "Surface #: ";
                cin  >> surface_num;
                cout << "Surface # entered: " << surface_num;
            }
            if (tooth_number>=22&&tooth_number<=27)
            {
                cout << "Anterior Tooth" << endl;
                cout << "Composite" << endl;
                cout << "Surface #: ";
                cin  >> surface_num;
                cout << "Surface # entered: " << surface_num;
            }
        }
    }
}
\$\endgroup\$
  • 7
    \$\begingroup\$ If you have a problem X and code Y to solve it, and code Y is terrible to look at, then it's more productive if you describe problem X, perhaps with a short excerpt of your current attempt, rather than make us reverse-engineer Y to figure out X and then resolve it. \$\endgroup\$ – Kerrek SB Oct 13 '13 at 21:10
  • 1
    \$\begingroup\$ Break the task into sub-tasks. Write one function for each sub-task. There also seems to be quite a lot of code duplication, functions will help with that too. \$\endgroup\$ – john Oct 13 '13 at 21:12
  • 1
    \$\begingroup\$ This looks like a school assignment to me. \$\endgroup\$ – edtheprogrammerguy Oct 13 '13 at 21:12
  • 3
    \$\begingroup\$ Not at all an answer to your question, but this seems to be a kind of problem better tackled by an scripting language such as Python. For the kind of code you are presenting, such a language would be vastly simpler to develop and debug. \$\endgroup\$ – Arrieta Oct 13 '13 at 21:12
  • 1
    \$\begingroup\$ You should always verify that any inputs were successfully read! (that's independent of the programming language, BTW) \$\endgroup\$ – Dietmar Kühl Oct 13 '13 at 21:14
5
\$\begingroup\$

A couple of style Issues jump out to me.

  1. mixed use of { braces. I think it is a matter of preference but I find that not using them for the if is confusing when you use them for the else if.

    if (cav_deep==1)
    {
        cout << "Temporary filling needed. Patient needs to come back for permanent      fill at later date." << endl;
    }
    else if (cav_deep==2)
    {
        cout << "Fillings Needed?: 1.Yes 2.No" << endl;
        cin  >> fillings;
        if (fillings==2)
        {
            cout << "Proceed to 'crowns'" << endl;
        }
       ///...
    
  2. I'm not a fan of using a paragraph length name for variables but you have define the variable names and then explained the abbreviation. Just name them a little more explicit and then you can read the code easier?

    bool posterior,         
         anterior;              
    int fillings;            // fillings: Y/N
    int tooth_number;        // tooth_number: 1-32
    int surface_num;         // surface_num should be 1-5
    int amalgum_or_composite;// amalgum or composite filling
    int is_cavity_deep;       
    
\$\endgroup\$
  • 1
    \$\begingroup\$ You could also mention that is_cavity_deep should be a bool. \$\endgroup\$ – Jamal Oct 14 '13 at 5:37
3
\$\begingroup\$

First thing I would do it separate the algo from the text. Two options for that: store the text into a map or use a config file.

Second thing is to avoid magic numbers: what do 6 or 32 mean. Declare const variables with these values and it will make your code more readable

Third thing is to check the user input "Fillings Needed?: 1.Yes 2.No". What happens if the user enter 3 or something different? You can create a function for that that would loop as long as the input is not correct.

Try to use enums for the different types of teeth.

Hope that helps :-)

\$\endgroup\$
3
\$\begingroup\$

First, don't use using namespace std: either fully qualify, or use "using declarations" (yours is a using directive, which imports the whole std namespace, usually a bad idea for the possible conflicts).

Second, use "\n" instead of endl: alternating cin and cout needs no intermediate flush, because the two are "tied".

As the others have pointed out, for variables, prefer descriptive names over comments.

\$\endgroup\$
2
\$\begingroup\$

Just for starters, the tooth number determines whether it is posterior or anterior. Put that into a function that could use a std::map<int, bool>. In the function, validate that the input is greater than 0 and less than 33. Or, look it up in the map and make sure you found it. If the tooth number is not in the map, then the input was either wrong, or the person has an extra tooth. What does your mother want to do when that happens?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy