# Drawing superellipses

Here is a piece of code that draws superellipses:

#include "std_lib_facilities.h"
#include <iostream>
#include "Simple_window.h"

#define PI 3.14159265359

template <typename T>
int sgn(T val);

vector<Point> generateSuperEllipse(double CoeffA, double CoeffB, double expN, double expM, int centX, int centY, double precision);
/*
Class: SuperEllipse
Each initialized object
generates a superellipse
using the parameters of the
constructor. The two member
functions draw the generated
points.
*/
class SuperEllipse{
public:
SuperEllipse(double CoeffA, double CoeffB, double expN, double expM, int centX, int centY, double precision){
// generate ellipse points
coordinates = generateSuperEllipse(CoeffA, CoeffB, expN, expM, centX, centY, precision);
}
// draw a Star-like pattern connecting superellipse's single point to the rest of its equally spaced points
void drawStar();
// draw a superellipse out of the generated points contained in private member coordinates
void drawSuperEllipse();
private:
vector<Point> coordinates;
};

//------------------------------------------------------------------------------------------------------------------------------
int main(){
try{
// Superellipse parameters
// exponents
double n = 2/3.;
double m = 2/3.;
// coefficient
double A = 200.0;
double B = 200.0;
// center point of the graph
const int centX = x_max() / 2.;
const int centY = y_max() / 2.;
// incrementation step
double di = 0.01;

SuperEllipse se(A, B, n, m, centX, centY, di);
se.drawSuperEllipse();
// push next button to see the star
se.drawStar();

}catch(exception& e){
cerr << e.what() <<endl;
getchar();
}catch(...){
cerr <<"Default exception!"<<endl;
getchar();
}
return 0;
}
//------------------------------------------------------------------------------------------------------------------------------
/*
Function: sgn()
Use: int singedOneorZero = sgn(anyType);
Returns either -1, 0 or 1 if the input
value is < 0 , == 0 or > =, respectivelly.
*/
template <typename T>
int sgn(T val){
return (T(0) < val) - (val < T(0));
}

/*
Function: generateSuperEllipse()
Use: vector<Point> generateSuperEllipse(double coeffA, double coeffB,
double expN, double expM,
int centX, int centY,
double precision)
This function generates a set of superellipse coordinates based
on the following parametric equations:
x(theta) = coeffA * pow(abs(cos(theta)), 2 / n) * sgn(cos(theta)) + centX;
y(theta) = coeffB * pow(abs(sin(theta)), 2 / m) * sgn(sin(theta)) + centY;
and returns a vector containing them.
*/
vector<Point> generateSuperEllipse(double coeffA, double coeffB, double expN, double expM, int centX, int centY, double precision = 0.01){
if(precision < 0.01 || precision > 1) error("Precision out of range!");

vector<Point> superEllipseCoordinates;
Point temp, sEllipseCoordinate;

for(double d = -1; d < 1; d += precision){
double theta = d * PI;
int x = coeffA * pow(abs((double)cos(theta)), (double)2.0 / expM) * sgn(cos(theta));
int y = coeffB * pow(abs((double)sin(theta)), (double)2.0 / expN) * sgn(sin(theta));
sEllipseCoordinate = Point(x + centX/2. , y + centY);
// check for duplicate points
if(sEllipseCoordinate != temp) superEllipseCoordinates.push_back(sEllipseCoordinate);
temp = sEllipseCoordinate;
}
return superEllipseCoordinates;
}

// Class Superellipse members implementation
/*
Member Function: drawStar()
Use: SuperEllipse sEobject.drawStar();
Generates a Simple_window
object and draws a star-like pattern
by connecting a single one of
points to the equally spaced remaining point.
Finally it attaches the drawing
to the window object.
*/
void SuperEllipse::drawStar(){
const int swWidth = 800;
const int swHeight = 800;
Simple_window sw(Point((x_max()- swWidth) / 2. , (y_max() - swHeight) / 2.),
swWidth, swHeight, "Chapter 12 Exercise 12");
// If the initial value of the counter: i is not divided by a number (100) it connects
// a lot of points and results in filling the superellipse
for(size_t i = coordinates.size() / 100; i < coordinates.size(); ++i){
for(size_t j = i+1; j < coordinates.size(); ++j){
Graph_lib::Line* star = new Line(Point(coordinates[0]),Point(coordinates[j]));
sw.attach(*star);
}
}
sw.wait_for_button();
}

/*
Member Function: drawSuperEllipse();
Use: SuperEllipse sEobject.drawSuperEllipse();
Generates a Simple_windoq object
and draws an open polyline of the
coordinates passed as parameter (in the case
superellipse). Finally it attaches the drawing
to the window object.
*/
void SuperEllipse::drawSuperEllipse(){
const int swWidth = 800;
const int swHeight = 800;
Simple_window sw(Point((x_max()- swWidth) / 2. , (y_max() - swHeight) / 2.),
swWidth, swHeight, "Chapter 12 Exercise 12");

Graph_lib::Open_polyline superEllipse;
for(size_t i = 0; i < coordinates.size(); ++i)  superEllipse.add(coordinates[i]);
sw.attach(superEllipse);
sw.wait_for_button();
}


## Drawing Results:

Superellipse:

Star-like pattern:

## Questions:

1. Is there a better way to implement the two drawing member function without incorporating the Simple_window sw object inside the functions (possibly passed as parameter or part of the class)?
2. Any remarks regarding class and member function implementation?
3. Any style improvement notes?
4. How can I delete the dynamically allocated memory in function drawStar()? (Should it be done at the end of the function or as part of an explicitly defined destructor?)

All the needed files for compilation are here. The FLTK can be found here.

• I started to write an answer. Is it possible to modify the code you link to? At the very least, the Window class needs a few modifications. – Armaghast Sep 7 '15 at 17:01
• @Armaghast in the above code all the included headers don't need any modification (specifically window is not used directly and the modification is not recommended ). If you have problems running it: I would be happy to help, as they are indeed some fine points that need attention. – Ziezi Sep 7 '15 at 17:05

## Don't wrap your entire program in a try block

It's generally not good to have the entire body of main be a try block. The reason is that if an exception is thrown, it's more difficult for the reader of the program to figure out which statements might have thrown. We know that none of the simple variable declarations throw, so it seems to me (although we don't know the contents of the two include files) that only the last three lines might throw.

## Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. It is particularly bad to put it into a header file, so please don't do that.

The constructor code is implemented in an odd two-part fashion with the constructor calling an external standalone function. Better would be to simply make the body of standalone function be the constructor body and eliminate the standalone function.

## Move loop invariants out of the loop

A number of factors within the loop are potentially calculated many times. Your loop could be rewritten like this:

expN = 2.0 / expN;
expM = 2.0 / expM;
precision *= PI;
for (double theta = -PI; theta < PI; theta += precision) {
int x = coeffA * pow(abs(cos(theta)), expM) * sgn(cos(theta));
int y = coeffB * pow(abs(sin(theta)), expN) * sgn(sin(theta));
superEllipseCoordinates.emplace_back(Point(x+centX/2., y+centY));
}


Note also that variable d is no longer needed and that I've used emplace_back with an inline constructor. There does not seem to be any real reason to look for duplicates or to name the value.

## Consider an alternative form for draw and drawStar

Rather than creating (and destroying) a Simple_window object within your code, it might be better to do that outside and have those functions return something useable by a Simple_window. For drawStar that might look like this:

Graph_lib::Open_polyline SuperEllipse::getPolyline()
{
Graph_lib::Open_polyline poly;
for (size_t i = 0; i < coordinates.size(); ++i)
return poly;
}


Using it would be like this:

const int swWidth = 800;
const int swHeight = 800;
Simple_window sw(Point((x_max() - swWidth) / 2., (y_max() - swHeight) / 2.),
swWidth, swHeight, "Chapter 12 Exercise 12");
sw.attach(se.getPolyline());
sw.wait_for_button();


## Don't leak memory

I don't know how your Graph_lib or Simple_window handle memory, but I see that drawStar() allocates memory with new but never frees it. This is suspicious and is probably a memory leak.

Beyond that, it's hard to say much because we don't know what's in the include files.

• getPolyline can't work like this: you are returning nothing, and adding return superEllipse; would be a bug. I suggest a dynamically allocation, thus returning a Graph_lib::Open_polyline*, and then let the Window take ownership of its child, as usual (which is currently not the case). – Armaghast Sep 7 '15 at 18:04
• @Armaghast: fixed now. Thanks for pointing that out. – Edward Sep 7 '15 at 18:07
• @Edward Firstly thank you for your answer! The only way I can think off for freeing the dynamically allocated memory in drawStar() is if I save the created objects in a container (vector for example) and then run a for loop with delete followed by indexed element. Is there a other/ better way to free that memory leak? – Ziezi Sep 7 '15 at 19:13
• Putting things in a container is probably the best way to do it. You could then return that container from the call (as I showed with getPolyline()) and not leak memory. – Edward Sep 7 '15 at 19:53
• Still doesn't work, as Graph_lib::Open_polylinecannot be copied. Using dynamic allocation + return a (smart) pointer is the way to go - those methods are widget factory after all; the Window should clean the memory afterwards. – Armaghast Sep 8 '15 at 8:49

This answer is a correction to Edward's one (comments are simply not enough).

I fully agree on the "Don't wrap your entire program in a try block", "Don't abuse using namespace std", "Move loop invariants out of the loop", and will not repeat those points.

You can keep generateSuperEllipse(), but make it a private & static method of SuperEllipse. Use member initialization:

SuperEllipse(double CoeffA, double CoeffB, double expN, double expM, int centX, int centY, double precision) :
coordinates(generateSuperEllipse(CoeffA, CoeffB, expN, expM, centX, centY, precision))  // generate ellipse points
{
}


Thus you can make make coordinates a const vector<Point>.

Shape generation

Indeed, the drawStar()/drawSuperEllipse() should not handle the creation of the window. Those methods should return something that looks like a Graph_lib::Open_polyline, and let the caller add the shape to the window he/she wants to.

However, Edward's solution isn't working. A Graph_lib::Open_polyline cannot be copied (which is frequent for Widget-like elements):

Shape(const Shape&) = delete;


You need to use dynamic allocation (better here than a move, due to polymorphism):

Graph_lib::Open_polyline* SuperEllipse::makePolyline()
{
Graph_lib::Open_polyline* poly = new Graph_lib::Open_polyline;
for (size_t i = 0; i < coordinates.size(); ++i)
return poly;
}


Note that I renamed it makePolyline(), due to the dynamic allocation, as it carry the notion the caller now own the Graph_lib::Open_polyline*

Of course, makeStar should be using a Open_polyline

Onto fixing the memory leak

Graph_lib::Open_polyline is a Shape, which behaves very closely to a Widget in your library.

In this case, the parent is responsible for cleaning up its children's memory (which, in turn clean up their children memory, etc.). The container you are talking about already exists - it's Window vector<Shape*> shapes;.

2 steps are thus needed:

• Windowshould have a method void attach(Shape* s); instead of void attach(Shape& s);

• ~Window() should destroy every elements of vector<Shape*> shapes;:

~Window()
{
for(auto it = shapes.begin(); it != shapes.end(); it++)
destroy *it;
}


I suggest doing the same with void attach(Widget& w); and the void detach(...) methods

Note: I am now enforcing the rule "the Window owns the Shapes", which wasn't the case before. Thus, it can change the behavior of old code using this library.

An underlying issue

I believe Shape should inherit Widget, and the Window-related classes should only deal with Widgets. (void detach(Widget& w); is declared, yet not defined, so I guess you're not using Widgets)

But we're now refactoring code outside of what was posted. I am unsure whether your code review is only about the SuperEllipse class that was posted, or also about the code you link to.

Further improvement: explicit ownership/automatic deallocation with C++11

An improvement over my proposed solution is makePolyline() / makeStar() should return a std::unique_ptr :

std::unique_ptr<Graph_lib::Open_polyline> SuperEllipse::makePolyline()
{
std::unique_ptr<Graph_lib::Open_polyline> poly(new  Graph_lib::Open_polyline>);
for (size_t i = 0; i < coordinates.size(); ++i)
return poly;
}


Likewise (assuming you keep Shapeand Widgetseparate), use std::vector<std::unique_ptr<Shape>> shapes; and void attach(std::unique_ptr<Widget> w); (as an overload) in Window: the memory is automatically cleaned up when ~Window() is called.

// non-used headers contained in std_lib_facilities.h, removed
// no explicit use of namespace std (except if its already in some of the headers connected to "Simple_window")
#include <cmath>
#include <stdexcept>
#include <string>
#include <vector>
#include <iostream>
#include "Simple_window.h"

#define PI 3.14159265359

class regularPolygon{
public:
regularPolygon(Point centerOfPolygon, int numbOfSides, int size)
: axisOfRotation(centerOfPolygon), numberOfRotations(numbOfSides), sizeOfSides(size)
{ generatePolygon(); }
// explicitly defined destructor that frees the dynamically allocated memory for the polygons
~regularPolygon()
{ for(auto it = generatedPolygons.begin(); it != generatedPolygons.end(); it++) delete *it; }
// member function returning a polygon object drawn using the vector: coordinates
Graph_lib::Polygon* makePolygon();
private:
Point axisOfRotation;
Point initial;
int numberOfRotations;
int sizeOfSides;

void rotateCoordinate(vector<Point>& rotated, Point& axisOfRotation, Point initial,
// wrapper function (if it's declared as static the definition gets underlined in red)
void generatePolygon();
// container holding the generated coordinates
vector<Point> coordinates;
// container holding the generated polygons used to free the dynamically alocated memory
// common to all instantiated objects
static vector<Graph_lib::Polygon *> generatedPolygons;
};

//-----------------------------------------------------------------------------------------------------------------------

int main(){

// simple_window object
const int swWidth = 800;
const int swHeight = 800;
Simple_window sw(Point((x_max()- swWidth) / 2. , (y_max() - swHeight) / 2.),
swWidth, swHeight, "Chapter 12 Exercise 11");

// error-handling only around the newly introduced objects
try{
// center (pivot) point
Point center(sw.x_max() / 2., sw.y_max() / 2.);
// Equilateral triangle
const int sideOfTriangle = 125;
regularPolygon equilateralTriangle(center, 3, sideOfTriangle);
sw.attach(*equilateralTriangle.makePolygon());
// Square
const int sizeOfSquare = sideOfTriangle;
regularPolygon square(center, 4,  sizeOfSquare);
sw.attach(*square.makePolygon());
// Pentagon
const int sizeOfPentagon = sideOfTriangle;
regularPolygon pentagon(center, 5, sizeOfPentagon);
sw.attach(*pentagon.makePolygon());

sw.wait_for_button();
// should I call the class makePolygon() destructor explicitly, here?
}catch(exception& e){
cerr << e.what() <<endl;
getchar();
}catch(...){
cerr <<"Default exception!"<<endl;
getchar();
}
return 0;
}
//-----------------------------------------------------------------------------------------------------------------------

/*
Member Function: rotateCoordinate()
Use: Private member function wrapped by the
function generatePolygon();
This function performs the same rotation on
a consevutively generated points, through recursion.
These points are saved in a vector, passed
by reference.
*/
void regularPolygon::rotateCoordinate(vector<Point>& rotated, Point& axisOfRotation, Point initial,
// base case: when all rotations performed return vector holding the rotated points
if(numberOfRotations <= 0) return;
else{
// apply transformation on the initial point
int x = cos(angRads) * (initial.x - axisOfRotation.x) - sin(angRads) * (initial.y - axisOfRotation.y) + axisOfRotation.x;
int y = sin(angRads) * (initial.x - axisOfRotation.x) + cos(angRads) * (initial.y - axisOfRotation.y) + axisOfRotation.y;
// save the result
rotated.push_back(Point(x, y));
// call the same function this time on the rotated point and decremented number of rotations
rotateCoordinate(rotated, axisOfRotation, Point(x,y), angRads, numberOfRotations -1);
}
}

/*
Member Function: generatePolygon()
Use: Private function used in the body
of the constructor to generate coordinates
for the polygon.
*/
void regularPolygon::generatePolygon(){
if(axisOfRotation.x < 0 || axisOfRotation.y < 0) error("Pivot point coordinates out of lower bound!");
if(numberOfRotations < 3) error("Not enough polygon sides!");
if(sizeOfSides < 0) error("Negative side lenght!");

coordinates.reserve(numberOfRotations);
// calculate the angles of rotations
angRads = PI / 180. * (360. / numberOfRotations);
// coordinates of first point shifted by the side length (it should be by the bisector (to be done latter))
initial.x = axisOfRotation.x - sizeOfSides;
initial.y = axisOfRotation.y - sizeOfSides;
// generate the coordinates of the polygon
}

/*
Member Function: makePolygon();
Use: Private function used to
create (return) a pointer to a polygon
object created from the generated
coordinates.
*/
Graph_lib::Polygon* regularPolygon::makePolygon(){
Graph_lib::Polygon* poly = new Graph_lib::Polygon;
for(size_t i = 0; i < coordinates.size(); ++i) poly->add(coordinates[i]);
// collected in the container to be deleted at the end
generatedPolygons.push_back(poly);
return poly;
}


The only problematic thing is to declare the function generatePolygon() as static, it results in red underline across all of the body of definition, containing the error message: "static members should belong to objects"