That's a nice wall of code you got there. Care to format it for us?
–
seheApr 1 '12 at 20:14

6

At the fourth line I think "This code is so bad, would I have to maintain it, I'd rather write it anew over the weekend." And thus endedth my attempt at answering your question.
–
sbiApr 1 '12 at 20:16

8

sorry but I'm not good at writing C/C++ code, and such comments unfortunately doesn't solve my problem at all. I'm still learning, as all of us..
–
ConradApr 1 '12 at 20:20

6

@ the OP: I would fire up a debugger and step through it.
–
pg1989Apr 1 '12 at 20:21

3

@Conrad: for starters, 1) please don't abuse #defines. (Line 4 of your source is definitely an abuse.) 2) The inline keyword should only be used sparingly if at all (the compiler will inline things if it feels like it; the inline keyword is not usually required.) 3) use meaningful names, and comment your code! (what does odl() do? What does fun() do?)
–
Li-aung YipApr 1 '12 at 20:26

2 Answers
2

The message means you've done something bad with dynamically allocated memory. Perhaps you freed an object twice, or wrote into memory beyond the beginning or end of an array-like dynamically allocated object.

On Linux, the tool valgrind may help pin-point the first place in your program's execution where it made a boo-boo.

By the way, your macro:

#define REP(i,n) for(int i=0;i<n;i++)

is poorly defined. The substitution of n should be parenthesized, because n could be an expression which has the wrong precedence with respect to the < operator. For instance: REP(i, k < m ? z : w). You want:

#define REP(var,n) for(int var=0;var<(n);var++)

The var reminds the programmer that this argument is a variable name, and not an arbitrary expression.

Even better (but still C style, not C++): #define REP(type,var,n) for(type var=0;var<(n);var++). The reason is that the use is more informative: REP(int,i,5) { printf("%d", i); }
–
MSaltersApr 2 '12 at 8:22

You can write a simple function dist calcDist(point,point) for this. You should probably move all the point definitions and associated functions to a separate header "point.h", again to keep things readable.

As for the memory issue: first, the arrays X_L and X_R are not really necessary. They contain the same data as X, so you can make them pointers to &(X[0]) and &(X[p) respectively. Y_L and Y_R are shuffled versions, so you do need to the arrays to copy data to. However, if you allocate them with new[], you are responsible for cleanup with delete[]. It looks like you can just use a std::vector<point> Y_L instead. No need to do bookkeeping, vector does that for you. Just call Y_L.push_back(Y[i]).