HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

User-defined class for a box

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
classuserforboxdefined

Problem

We have just learned about classes. Please help me improve this program. I created a class called Box with members that represent its dimensions. Then I calculate the volume and surface area and print it.

#include 

using std::cout;
using std::endl;

class Box
{
public:
    Box(){};
    double m_Length;
    double m_Width;
    double m_Height;
private:
};

int main()
{
    Box box1;
    Box box2;

    double boxVolume(0.0);
    double surfaceArea(0.0);

    box1.m_Height = 18.0;
    box1.m_Length = 78.0;
    box1.m_Width = 24.0;

    boxVolume = box1.m_Height * box1.m_Length * box1.m_Width;
    surfaceArea = 2.0 * ((box1.m_Height * box1.m_Width) + (box1.m_Length * box1.m_Height) + (box1.m_Length * box1.m_Width));

    cout << "Volume of box1 = " << boxVolume << endl;
    cout << "Surface Area of box1 = " << surfaceArea << endl;
}

Solution

The Box class has all public members and doesn't have any behavior (no methods). As such, it's a missed opportunity and could have been a simple struct.

Even worse, what's the point of a Box if any of length or width or height is undefined? These should all be required to construct a meaningful representation of a box.

The calculation of the surface and volume are inside the main method. This is not convenient, because you would have to re-type the formula every time you want to calculate this for every single box.

To remedy the above:

  • make the length, width, height variables private: code outside of the class should not modify them



  • make the length, width, height required at construction time: a box without these parameters doesn't make sense, and is not good modelization



  • I would go as far as making these variables constant: in real life, the width / height / length of a physical box is usually not editable



  • add surface and volume calculation logic as methods on the box, so that they can be reused for any Box object



A better interface for a Box would be:

class Box
{
public:
    Box(double length, double width, double height);
    inline double volume();
    inline double surface();
private:
    const double m_Length;
    const double m_Width;
    const double m_Height;
};


With implementation:

Box::Box(double length, double width, double height) :
    m_Length(length), m_Width(width), m_Height(height) {}

double Box::volume()
{
    return m_Length * m_Width * m_Height;
}

double Box::surface()
{
    return 2 * (m_Length * m_Width + m_Width * m_Height + m_Height * m_Length);
}


With the above, the main method can be rewritten as:

int main()
{
    Box box1(78, 24, 18);

    cout << "Volume of box1 = " << box1.volume() << endl;
    cout << "Surface Area of box1 = " << box1.surface() << endl;
}


Note that since the implementation of the constructor and the surface and volume methods are so simple,
it might be tempting to write them inside the declaration of the class, like this:

class Box
{
public:
    Box(double length, double width, double height) : m_Length(length), m_Width(width), m_Height(height) {}
    double volume() { return m_Length * m_Width * m_Height; }
    double surface() { return 2 * (m_Length * m_Width + m_Width * m_Height + m_Height * m_Length); }
private:
    double m_Length;
    double m_Width;
    double m_Height;
};


But this is a not a good practice.
The good practice is to cleanly separate the interface declaration from the implementation.
However,
while you're learning,
if this last version is easier for you,
then it's ok, you can do it this way, for now.
Just remember later as your understanding evolves,
to try to separate interface from implementation.

Finally, some other minor issues:

No need to write 18.0 when assigning to a double variable,
you could write simply 18.

No need to put parentheses around multiplication in (a b) + (c d),
a b + c d is equivalent.

Code Snippets

class Box
{
public:
    Box(double length, double width, double height);
    inline double volume();
    inline double surface();
private:
    const double m_Length;
    const double m_Width;
    const double m_Height;
};
Box::Box(double length, double width, double height) :
    m_Length(length), m_Width(width), m_Height(height) {}

double Box::volume()
{
    return m_Length * m_Width * m_Height;
}

double Box::surface()
{
    return 2 * (m_Length * m_Width + m_Width * m_Height + m_Height * m_Length);
}
int main()
{
    Box box1(78, 24, 18);

    cout << "Volume of box1 = " << box1.volume() << endl;
    cout << "Surface Area of box1 = " << box1.surface() << endl;
}
class Box
{
public:
    Box(double length, double width, double height) : m_Length(length), m_Width(width), m_Height(height) {}
    double volume() { return m_Length * m_Width * m_Height; }
    double surface() { return 2 * (m_Length * m_Width + m_Width * m_Height + m_Height * m_Length); }
private:
    double m_Length;
    double m_Width;
    double m_Height;
};

Context

StackExchange Code Review Q#66389, answer score: 4

Revisions (0)

No revisions yet.