0
\$\begingroup\$

My code is working well and uses a lot of mechanics in C++, however, I want to know if there are any ways to make my code better, some possible problems in use. this is a simple program and I started yesterday.

What other mechanics could I implement to make the code better? And where can I find more information about those mechanics?

// IQ rand generator by ashar rahman

#include "stdafx.h"         //includes rand and other related commands
#include <stdlib.h>         //something windows wants
#include <iostream>         //allows for c++ IO interaction
#include <time.h>           //allows for srand time manipulation
#include <cstdlib>          //standard c++ libraryu

using namespace std;        //tells compiler to use std for all namespaces ex. std::cout --> cout

class IQClass{              //class containing all IQ generating, name and multiplyer selection related functions.
public:

    int y;                  //random IQ storage integer 
    long int a;             //answer after  multiplyer storage integer, long for larger numerical answers
    int m;                  //multiplyer container
    char name[80];          //char array container for user name supporting upto 80 bytes

    void nameInput() {      //function including input prompt for user name
        cout << "What is your name?\n\n";       //prints 'What is your name?' with cout \n are line returns
            cin >> name;                        //allows user to input name with cin
    };

    void multiplierSelection() {        //function contains input prompt for multiplier
        cout << "\nWhat should the multiplier be?\n\n ";                    //prints
            cin >> m;                                                           //asks for input to fill m variable
    }

    void calc() {   //function contains equasion to find answer of y*m
        a = y*m;                                        //equasion
    }

    void print() {                      //function contains printing of answer, random IQ, multiplyer, and name, endl is another line return type
        cout << name << "'s IQ is probably: " << y << endl << "That times:     " << m << " is " << a << endl;
    }

    void randGen() {            //creates random sequence
        y = rand() % 160;           //out of 160 finds random value
    };



private:


};

    IQClass IC;                 //changes identifer for IQCLass ex.              IQClass.nameInput(); ---> IC.nameInput

int main()
{
     srand(time(NULL));             //generates random seed once for rand to use


     IC.nameInput();                                            //allows user to input name
     IC.multiplierSelection();                              //allows user to select multiplier

        for (int x = 0; x < 100;) {             //prints 100 random IQs for user and the answers relative to the multiplier
            IC.randGen();                               //generates random sequence
            IC.calc();                      //calculates product
            IC.print();                     //prints answer(s)
         x = x + 1 ;                
     };
    return 0;           //ends program with return code of 0 
}

As you can see I have annotated everything for maximum explanation and so I can review/remember it if I forget. Are any of the comments wrong and if so, how can they be improved?

\$\endgroup\$
1
  • 1
    \$\begingroup\$ std::string for "name", please. std::cin >> name is prone to dangerous buffer overflows. \$\endgroup\$
    – jvb
    Commented Jul 18, 2017 at 6:59

2 Answers 2

1
\$\begingroup\$

You don't need comments explaining the standard library headers - credit your readers with enough knowledge to know what those lines are for.

Most of the other comments could be removed (in some cases, we can use better names for variables such as y, a and m to communicate the meaning instead).

There's nothing to stop overrun of the name variable - prefer to use std::string instead, as it will automatically resize as required.

Avoid using namespace std - importing all names of a namespace is a bad habit to get into, and can cause surprise when names like begin and size are in the global namespace. Get used to using the namespace prefix (std is intentionally very short), or importing just the names you need into the smallest reasonable scope.

IC doesn't need to be a global variable - it can be a local to main().

IQ scores are usually defined as a Normal distribution of mean 100 and standard deviation 15. You can use a std::normal_distribution (from <random>) to generate values that fit.

\$\endgroup\$
0
\$\begingroup\$
  1. All the class variables should be private. If you need to access them from outside the class, do it with a public getter/setter functions.

  2. Only class functions that must be accessed from outside the class should be public.

  3. In the for loop you have 3 calls to functions from class IQClass. You should extract the 3 calls to a new class-function and call it each iteration.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.