How would you optimise this code?

[Maniax]

Hello :) I'm trying to be more efficient when writing code so here's an extract.
I feel like there's a simpler way to write this code but I'm not sure... What would you suggest to make the program faster and use up less memory?

Header file.h

class SQUARE
{
private:
    double side;
    double area;
    double diagonal;
    double perimeter;
    bool notdone;
    void update();
public:
    void populate();
    void print();
};

Definition file.cpp

void SQUARE::populate()
{
    using namespace std;
    
    const char *error = "Pelease enter a number: ";
    notdone = 1;
    
    input(side, "the side of the square", error);
    if (side > 0)
        update();
    if (notdone)
    {
        input(area, "the area of the square", error);
        if (area > 0)
            update();
    }
    if (notdone)
    {
        input(perimeter, "the perimete of the square", error);
        if (perimeter > 0)
            update();
    }
    if (notdone)
    {
        input(diagonal, "the diagonal of the square", error);
        if (diagonal > 0)
            update();
    }
    if (notdone)
        cout << "No data entered. Aborting." << endl;
}

void SQUARE::update()     //Would it be better if each was split into difference functions?
{                         //Eg. int area (int a, int b) {return a * b;}
    if (side > 0)
    {
        area = side * side;
        perimeter = 4 * side;
        diagonal = sqrt(2 * side * side);
    }
    else if (area > 0)
    {
        side = sqrt(area);
        perimeter = 4 * side;
        diagonal = sqrt(2 * side * side);
    }
    else if (perimeter > 0)
    {
        side = perimeter / 4;
        area = side * side;
        diagonal = sqrt(2 * side * side);
    }
    else if (diagonal > 0)
    {
        side = sqrt((diagonal * diagonal)/2);
        area = side * side;
        perimeter = 4 * side;
    }
    notdone = false;
}

Main.cpp

if (choice == 1)
        {
            SQUARE *s = new SQUARE;
            s->populate();
            s->print();
            delete [] s;
        }

[codekiddy]

dont spend to much time with optimizations cos mostly compiler does that for you, but if you wanna know some hints:

you function void SQUARE::populate()
could be writen more efficient using switch statement.
not because that would produce faster code but the code it self will be more readable.

you said:

//Would it be better if each was split into difference functions?
{                         //Eg. int area (int a, int b) {return a * b;} 

no.

because one having one function is better then alot functions in a class but that depends on wether you need more functions or not.

Your code:

if (choice == 1)
        {
            SQUARE *s = new SQUARE;
            s->populate();
            s->print();
            delete [] s;
        }

You are doing a terrible mistake here!
remember:
how many new so many deletehow many new[] so many delete[]

a you just called delete[] for new which is very wrong.


one more thing about afficient code.
do not alocate small objects dinamicaly like this your class class or POD types(build-in types)
because dinamic alocation and indirection takes alot more time then stack alocation and indirection.
dinamic alloc is used for big objects.

do no perform any optimizatioin on your code until oyu finish your project or until is not large enough.
the time involved in optimization mostly return nothing or few bytes or rew seconds which isn't worth allways.

cheers!

[Maniax]

Thanks for the tips :) made a few changes according to your advice!

[mik2718]

Efficiency:

Memory:
You don't need all of side, area diagonal, perimeter since all of them can be derived from just one. Maybe just store side and calculate the others as required.

Speed:
You may be able to save a few billionths of a second by storing pre-calculated area, diagonal and perimeter from side (as you have done)

Memory and speed are often opposites in gaining efficiency. Speed efficient algorithms often store pre-calculated intermediate values so as to make them faster.