patterncppMinor
Striped closed poly-line
Viewed 0 times
stripedclosedlinepoly
Problem
I'm working on
stripedpoly.cpp:
stripedpoly.h:
```
// Helper functions
static void generatePoints(vector& p){
p.push_back(Point(50,50));
p.push_back(Point(200,50));
p.push_back(Point(250,100));
p.push_back(Point(200,200));
p.push_back(Point(100,225));
p.push_back(Point(50,200));
p.push_back(Point(25,100));
}
// compare function for sorting by the x-coordinates of a Point
struct XLessThan{
inline bool operator()(Point& p1, Point& p2){ return (p1.x p1.y) std::swap(start, end);
yMin = start.y;
yMax = end.y;
}
bool operator= 0 && s = 0 && t polyEdges;
Point first = point(0);
Point last = point(number_of_points()-1);
polyEdges.push_back(Edge(last, first));
for (size_t i = 1; i polyPoints2;
for (size_t i = 0; i intersections;
for (size_t i = 0; i < polyEdges.size(); ++i){
for (size_t j = polyEdges[i].yMin; j < polyEdges[i].yMax; j += spacing){
Edge horizontal(Point(polyXmin, j), Point(polyXmax, j));
// check if polyEdge not horizontal
if (polyEd
class Striped_polyline which represents a closed poly-line filled with equidistant horizontal lines1:stripedpoly.cpp:
#include
#include
#include
#include "Graph.h"
#include "Simple_window.h"
#include "stripedpoly.h"
int main(){
Point tl(x_max()/2,0);
int width = 700;
int height = 700;
string label = "Striped Closed poly-line";
Simple_window sw(tl, width, height, label);
try{
// generate points for the poly-line
vector polyPoints;
generatePoints(polyPoints);
// create a poly-line
Striped_closed_polyline scp;
for (auto it = polyPoints.begin(); it != polyPoints.end(); ++it) scp.add(*it);
scp.set_spacing(5);
sw.attach(scp);
sw.wait_for_button();
}catch(exception& e){
cerr << e.what() << endl;
getchar();
}catch(...){
cerr <<"Default exception!"<< endl;
getchar();
}
}stripedpoly.h:
```
// Helper functions
static void generatePoints(vector& p){
p.push_back(Point(50,50));
p.push_back(Point(200,50));
p.push_back(Point(250,100));
p.push_back(Point(200,200));
p.push_back(Point(100,225));
p.push_back(Point(50,200));
p.push_back(Point(25,100));
}
// compare function for sorting by the x-coordinates of a Point
struct XLessThan{
inline bool operator()(Point& p1, Point& p2){ return (p1.x p1.y) std::swap(start, end);
yMin = start.y;
yMax = end.y;
}
bool operator= 0 && s = 0 && t polyEdges;
Point first = point(0);
Point last = point(number_of_points()-1);
polyEdges.push_back(Edge(last, first));
for (size_t i = 1; i polyPoints2;
for (size_t i = 0; i intersections;
for (size_t i = 0; i < polyEdges.size(); ++i){
for (size_t j = polyEdges[i].yMin; j < polyEdges[i].yMax; j += spacing){
Edge horizontal(Point(polyXmin, j), Point(polyXmax, j));
// check if polyEdge not horizontal
if (polyEd
Solution
Don't use
The lack of
-
Having it localized to one translation unit is one thing, but putting it into a header propogates it to whoever chooses to include your header. This is a Bad Thing™ in of itself.
-
Many trivial programs will not suffer from name conflicts; but a program that deals with shapes and drawing increases the chances you'll have a name conflict like
-
Using a third party library (if you are) increases the chances of conflicts exponentially. If you wrote it yourself, consider a namespace akin to
Where are your newlines and spaces?
Your code is incredibly hard to read. I'm not going to tell you specifically how you should format your code, but at least consider formatting it to increase readability.
Explain your algorithm; choose better variable names
Even putting the name of the algorithm in a comment will give the reader enough context to figure it out.
Redundancy
You have wrapper functions that pass arguments along but this is pointless. Why duplicate the interface? As you can imagine, the more granular you get, the more bloated your interface will be.
using namespace std;. SeriouslyThe lack of
std:: implies you have using namespace std; in a header somewhere. This is bad for the following reasons:-
Having it localized to one translation unit is one thing, but putting it into a header propogates it to whoever chooses to include your header. This is a Bad Thing™ in of itself.
-
Many trivial programs will not suffer from name conflicts; but a program that deals with shapes and drawing increases the chances you'll have a name conflict like
distance or vector.-
Using a third party library (if you are) increases the chances of conflicts exponentially. If you wrote it yourself, consider a namespace akin to
std or sfml.Where are your newlines and spaces?
Your code is incredibly hard to read. I'm not going to tell you specifically how you should format your code, but at least consider formatting it to increase readability.
emplace_back over push_backpush_back will delegate to emplace_back. The advantage emplace_back has over push_back is that it will forward the arguments to the constructor, so you don't have to specify the type.polyEdges.emplace_back(last, first);Explain your algorithm; choose better variable names
intersectPoint is incredibly difficult to read. p0_x is easy to deduce, but what is s1_x? What is this line doing?s = (-s1_y * (p0_x - p2_x) + s1_x * (p0_y - p2_y)) / (-s2_x * s1_y + s1_x * s2_y);Even putting the name of the algorithm in a comment will give the reader enough context to figure it out.
Redundancy
You have wrapper functions that pass arguments along but this is pointless. Why duplicate the interface? As you can imagine, the more granular you get, the more bloated your interface will be.
Code Snippets
polyEdges.emplace_back(last, first);s = (-s1_y * (p0_x - p2_x) + s1_x * (p0_y - p2_y)) / (-s2_x * s1_y + s1_x * s2_y);Context
StackExchange Code Review Q#107419, answer score: 7
Revisions (0)
No revisions yet.