We’re rewarding the question askers & reputations are being recalculated! Read more.
17 replaced http://stackoverflow.com/ with https://stackoverflow.com/
source | link

This isn't done automatically for compiler generated destructors (see an explanation herehere).

like that then1)

void compareAreasOffigures(const Shape* a, const Shape* b);

or even better

void compareAreasOffigures(const Shape& a, const Shape& b);

Note: Do that as soon as possible in your class designs, adding later is much harder than removing that constraint in a later refactoring or override.

Prefer const variable definitions over using preprocessor macros

Instead of

 #define M_PI       3.14159265358979323846

rather use

const double M_PI = 3.14159265358979323846;

or

constexpr double M_PI = 3.14159265358979323846;

for sake of type safety and clarity.

Don't use srand(), rand() with modern c++

Use a better random number generator than

srand(time(0));

// ...

rand();

There are various random generators available with the current standard in the numerics library.

Don't use == to compare floating point values

Don't use equality comparison for floating point types like

if (a->area() == b->area())

it's unlikely these will be exactly equal.

Rather test against std::numeric_limits::epsilon, there's a comprehensive example in the linked documentation.

Your purpose of using _USE_MATH_DEFINES is unclear and inconsistent

Your use of _USE_MATH_DEFINES and defining M_PI yourself is inconsistent.

The purpose of #define _USE_MATH_DEFINES is to use the predefined constants from cmath or math.h, but you define M_PI yourself anyways.

As mentioned herehere it should be defined before any of the #include statements to guarantee it works

#define _USE_MATH_DEFINES
#include <iostream>
#include <cstdlib>
#include <ctime>
#include <cmath>
#include <vector>

since you cannot know if any of the other header files already includes cmath or math.h.

The highlight of your solution

After so many points of critique, I want to emphasize one bonus point of your solution:

You weren't trapped by the Square is not a Rectangle confusion and provided unrelated classes for these (same for Circle is not an Ellipse).

Beginners often use inheritance like

class Rectangle {
protected:
    float sideA, sideB;
};

class Square : public Rectangle {
};

and run into trouble to keep the class member variables consistent (here's a more detailed resource).


1

This isn't done automatically for compiler generated destructors (see an explanation here).

like that then1)

void compareAreasOffigures(const Shape* a, const Shape* b);

or even better

void compareAreasOffigures(const Shape& a, const Shape& b);

Note: Do that as soon as possible in your class designs, adding later is much harder than removing that constraint in a later refactoring or override.

Prefer const variable definitions over using preprocessor macros

Instead of

 #define M_PI       3.14159265358979323846

rather use

const double M_PI = 3.14159265358979323846;

or

constexpr double M_PI = 3.14159265358979323846;

for sake of type safety and clarity.

Don't use srand(), rand() with modern c++

Use a better random number generator than

srand(time(0));

// ...

rand();

There are various random generators available with the current standard in the numerics library.

Don't use == to compare floating point values

Don't use equality comparison for floating point types like

if (a->area() == b->area())

it's unlikely these will be exactly equal.

Rather test against std::numeric_limits::epsilon, there's a comprehensive example in the linked documentation.

Your purpose of using _USE_MATH_DEFINES is unclear and inconsistent

Your use of _USE_MATH_DEFINES and defining M_PI yourself is inconsistent.

The purpose of #define _USE_MATH_DEFINES is to use the predefined constants from cmath or math.h, but you define M_PI yourself anyways.

As mentioned here it should be defined before any of the #include statements to guarantee it works

#define _USE_MATH_DEFINES
#include <iostream>
#include <cstdlib>
#include <ctime>
#include <cmath>
#include <vector>

since you cannot know if any of the other header files already includes cmath or math.h.

The highlight of your solution

After so many points of critique, I want to emphasize one bonus point of your solution:

You weren't trapped by the Square is not a Rectangle confusion and provided unrelated classes for these (same for Circle is not an Ellipse).

Beginners often use inheritance like

class Rectangle {
protected:
    float sideA, sideB;
};

class Square : public Rectangle {
};

and run into trouble to keep the class member variables consistent (here's a more detailed resource).


1

This isn't done automatically for compiler generated destructors (see an explanation here).

like that then1)

void compareAreasOffigures(const Shape* a, const Shape* b);

or even better

void compareAreasOffigures(const Shape& a, const Shape& b);

Note: Do that as soon as possible in your class designs, adding later is much harder than removing that constraint in a later refactoring or override.

Prefer const variable definitions over using preprocessor macros

Instead of

 #define M_PI       3.14159265358979323846

rather use

const double M_PI = 3.14159265358979323846;

or

constexpr double M_PI = 3.14159265358979323846;

for sake of type safety and clarity.

Don't use srand(), rand() with modern c++

Use a better random number generator than

srand(time(0));

// ...

rand();

There are various random generators available with the current standard in the numerics library.

Don't use == to compare floating point values

Don't use equality comparison for floating point types like

if (a->area() == b->area())

it's unlikely these will be exactly equal.

Rather test against std::numeric_limits::epsilon, there's a comprehensive example in the linked documentation.

Your purpose of using _USE_MATH_DEFINES is unclear and inconsistent

Your use of _USE_MATH_DEFINES and defining M_PI yourself is inconsistent.

The purpose of #define _USE_MATH_DEFINES is to use the predefined constants from cmath or math.h, but you define M_PI yourself anyways.

As mentioned here it should be defined before any of the #include statements to guarantee it works

#define _USE_MATH_DEFINES
#include <iostream>
#include <cstdlib>
#include <ctime>
#include <cmath>
#include <vector>

since you cannot know if any of the other header files already includes cmath or math.h.

The highlight of your solution

After so many points of critique, I want to emphasize one bonus point of your solution:

You weren't trapped by the Square is not a Rectangle confusion and provided unrelated classes for these (same for Circle is not an Ellipse).

Beginners often use inheritance like

class Rectangle {
protected:
    float sideA, sideB;
};

class Square : public Rectangle {
};

and run into trouble to keep the class member variables consistent (here's a more detailed resource).


1

16 replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
source | link
As mentioned by @Tamoghna Chowdhury@Tamoghna Chowdhury before.
As mentioned by @Tamoghna Chowdhury before.
As mentioned by @Tamoghna Chowdhury before.
15 added 194 characters in body
source | link

like that then:

void compareAreasOffigures(const Shape* a, const Shape* b);

or even better

void compareAreasOffigures(const Shape& a, const Shape& b);

Note: Do that as soon as possible in your class designs, adding later is much harder than removing that constraint in a later refactoring or override.

Prefer const variable definitions over using preprocessor macros

Instead of

 #define M_PI       3.14159265358979323846

rather use

const double M_PI = 3.14159265358979323846;

or

constexpr double M_PI = 3.14159265358979323846;

for sake of type safety and clarity.

Don't use srand(), rand() with modern c++

Use a better random number generator than

srand(time(0));

// ...

rand();

There are various random generators available with the current standard in the numerics library.

Don't use == to compare floating point values

Don't use equality comparison for floating point types like

if (a->area() == b->area())

it's unlikely these will be exactly equal.

Rather test against std::numeric_limits::epsilon, there's a comprehensive example in the linked documentation.

Your purpose of using _USE_MATH_DEFINES is unclear and inconsistent

Your use of _USE_MATH_DEFINES and defining M_PI yourself is inconsistent.

The purpose of #define _USE_MATH_DEFINES is to use the predefined constants from cmath or math.h, but you define M_PI yourself anyways.

As mentioned here it should be defined before any of the #include statements to guarantee it works

#define _USE_MATH_DEFINES
#include <iostream>
#include <cstdlib>
#include <ctime>
#include <cmath>
#include <vector>

since you cannot know if any of the other header files already includes cmath or math.h.

The highlight of your solution

After so many points of critique, I want to emphasize one bonus point of your solution:

You weren't trapped by the Square is not a Rectangle confusion and provided unrelated classes for these (same for Circle is not an Ellipse).

Beginners often use inheritance like

class Rectangle {
protected:
    float sideA, sideB;
};

class Square : public Rectangle {
};

and run into trouble to keep the class member variables consistent (here's a more detailed resource).

1)
void compareAreasOffigures(const Shape* a, const Shape* b);

or even better

void compareAreasOffigures(const Shape& a, const Shape& b);

Note: Do that as soon as possible in your class designs, adding later is much harder than removing that constraint in a later refactoring or override.

Prefer const variable definitions over using preprocessor macros

Instead of

 #define M_PI       3.14159265358979323846

rather use

const double M_PI = 3.14159265358979323846;

or

constexpr double M_PI = 3.14159265358979323846;

for sake of type safety and clarity.

Don't use srand(), rand() with modern c++

Use a better random number generator than

srand(time(0));

// ...

rand();

There are various random generators available with the current standard in the numerics library.

Don't use == to compare floating point values

Don't use equality comparison for floating point types like

if (a->area() == b->area())

it's unlikely these will be exactly equal.

Rather test against std::numeric_limits::epsilon, there's a comprehensive example in the linked documentation.

Your purpose of using _USE_MATH_DEFINES is unclear and inconsistent

Your use of _USE_MATH_DEFINES and defining M_PI yourself is inconsistent.

The purpose of #define _USE_MATH_DEFINES is to use the predefined constants from cmath or math.h, but you define M_PI yourself anyways.

As mentioned here it should be defined before any of the #include statements to guarantee it works

#define _USE_MATH_DEFINES
#include <iostream>
#include <cstdlib>
#include <ctime>
#include <cmath>
#include <vector>

since you cannot know if any of the other header files already includes cmath or math.h.

The highlight of your solution

After so many points of critique, I want to emphasize one bonus point of your solution:

You weren't trapped by the Square is not a Rectangle confusion and provided unrelated classes for these (same for Circle is not an Ellipse).

Beginners often use inheritance like

class Rectangle {
protected:
    float sideA, sideB;
};

class Square : public Rectangle {
};

and run into trouble to keep the class member variables consistent (here's a more detailed resource).


1As mentioned by @Tamoghna Chowdhury before.

like that then:

void compareAreasOffigures(const Shape* a, const Shape* b);

or even better

void compareAreasOffigures(const Shape& a, const Shape& b);

Note: Do that as soon as possible in your class designs, adding later is much harder than removing that constraint in a later refactoring or override.

Prefer const variable definitions over using preprocessor macros

Instead of

 #define M_PI       3.14159265358979323846

rather use

const double M_PI = 3.14159265358979323846;

or

constexpr double M_PI = 3.14159265358979323846;

for sake of type safety and clarity.

Don't use srand(), rand() with modern c++

Use a better random number generator than

srand(time(0));

// ...

rand();

There are various random generators available with the current standard in the numerics library.

Don't use == to compare floating point values

Don't use equality comparison for floating point types like

if (a->area() == b->area())

it's unlikely these will be exactly equal.

Rather test against std::numeric_limits::epsilon, there's a comprehensive example in the linked documentation.

Your purpose of using _USE_MATH_DEFINES is unclear and inconsistent

Your use of _USE_MATH_DEFINES and defining M_PI yourself is inconsistent.

The purpose of #define _USE_MATH_DEFINES is to use the predefined constants from cmath or math.h, but you define M_PI yourself anyways.

As mentioned here it should be defined before any of the #include statements to guarantee it works

#define _USE_MATH_DEFINES
#include <iostream>
#include <cstdlib>
#include <ctime>
#include <cmath>
#include <vector>

since you cannot know if any of the other header files already includes cmath or math.h.

The highlight of your solution

After so many points of critique, I want to emphasize one bonus point of your solution:

You weren't trapped by the Square is not a Rectangle confusion and provided unrelated classes for these (same for Circle is not an Ellipse).

Beginners often use inheritance like

class Rectangle {
protected:
    float sideA, sideB;
};

class Square : public Rectangle {
};

and run into trouble to keep the class member variables consistent (here's a more detailed resource).

like that then1)

void compareAreasOffigures(const Shape* a, const Shape* b);

or even better

void compareAreasOffigures(const Shape& a, const Shape& b);

Note: Do that as soon as possible in your class designs, adding later is much harder than removing that constraint in a later refactoring or override.

Prefer const variable definitions over using preprocessor macros

Instead of

 #define M_PI       3.14159265358979323846

rather use

const double M_PI = 3.14159265358979323846;

or

constexpr double M_PI = 3.14159265358979323846;

for sake of type safety and clarity.

Don't use srand(), rand() with modern c++

Use a better random number generator than

srand(time(0));

// ...

rand();

There are various random generators available with the current standard in the numerics library.

Don't use == to compare floating point values

Don't use equality comparison for floating point types like

if (a->area() == b->area())

it's unlikely these will be exactly equal.

Rather test against std::numeric_limits::epsilon, there's a comprehensive example in the linked documentation.

Your purpose of using _USE_MATH_DEFINES is unclear and inconsistent

Your use of _USE_MATH_DEFINES and defining M_PI yourself is inconsistent.

The purpose of #define _USE_MATH_DEFINES is to use the predefined constants from cmath or math.h, but you define M_PI yourself anyways.

As mentioned here it should be defined before any of the #include statements to guarantee it works

#define _USE_MATH_DEFINES
#include <iostream>
#include <cstdlib>
#include <ctime>
#include <cmath>
#include <vector>

since you cannot know if any of the other header files already includes cmath or math.h.

The highlight of your solution

After so many points of critique, I want to emphasize one bonus point of your solution:

You weren't trapped by the Square is not a Rectangle confusion and provided unrelated classes for these (same for Circle is not an Ellipse).

Beginners often use inheritance like

class Rectangle {
protected:
    float sideA, sideB;
};

class Square : public Rectangle {
};

and run into trouble to keep the class member variables consistent (here's a more detailed resource).


1As mentioned by @Tamoghna Chowdhury before.

14 added 182 characters in body
source | link
13 added 182 characters in body
source | link
12 added 182 characters in body
source | link
11 added 560 characters in body
source | link
10 added 560 characters in body
source | link
9 deleted 96 characters in body
source | link
8 added 158 characters in body
source | link
7 added 158 characters in body
source | link
6 added 916 characters in body
source | link
5 added 421 characters in body
source | link
4 added 421 characters in body
source | link
3 added 362 characters in body
source | link
2 added 194 characters in body
source | link
1
source | link