Caveat: Porting C++ code from non-class to class can get you in trouble

[Ramon F Herrera]

(newbie alert)

On several occasions my C++ code (sans class stuff) is working
perfectly, but being notorious for getting meeself in trouble, I
embark on a project to "classify" my code.

This is a previous example, a learned lesson:

Pre-Class Code:

for_each(m1, m2, &rhs_callback);

Post-Class code:

for_each(m1, m2, boost::bind(&Evaluate::rhs_callback, this, _1));

I don't even pretend to understand what's going there, I simply copy &
pasted the snippet, after thanking profusely the kind poster who
provided the black-box magic recipe. (it was in the Boost NG).

The following is my latest code that is in class-related trouble:

Pre-Class code:

(global)
struct point {
double x;
double y;
};

list<point> pointKeeper;

(inside class)
bool x_comparison(const point& pt1, const point& pt2)
{
return pt1.x < pt2.x;
}

pointKeeper.sort(x_comparison); // This won't compile :-(

I have tried things like:

pointKeeper.sort(ClassName::x_comparison);

But it won't compile.

Thanks for your expert assistance...

-Ramon

 

[Ramon F Herrera]

> I am not sure if this solves all your problems, post a complete example
> and error messages next time!
>
> hth
> Paavo

I suspected that would be the case, was just trying to save disk space
to Google, the poor thing :^D

Will read & try your suggestion...

-Ramon

 

[Ramon F Herrera]

(e-mail address removed):













> If you continue to rely on "black magic", don't be surprised
> when the code ceases to work at some Thursday night ;-)

I know! I actually open the black box once in a while, try not to be
too intimidated, and sure enough at some point it has no resource but
give up its mystery. Black boxes (should we call them "holes"?) are
continuously being created -and eliminated- in my code.

-Ramon

 

[Ramon F Herrera]

> post a complete example and error messages next time!

> I'm guessing in the above with very little information
> to go on, but there you go.

Ask and Thou Shalt Receive:

http://patriot.net/~ramon/dot-cpp-file.png
http://patriot.net/~ramon/dot-h-file.png

Error Messages with and without the ampersand operator:

http://patriot.net/~ramon/ErrorMess.png

Thanks!

-Ramon

 

[Ramon F Herrera]

Daniel T. ha scritto:




Or you just overload operator< for "point" and use the simple version of
sort() which doesn't take a predicate argument.

struct point {
  double x;
  double y;
  bool operator<(const point &rhs) const;

};

pointKeeper.sort();

That's a clever Hack, Hackl! :^D

-RFH

 

[Ramon F Herrera]

C++ is not a purely Object-Oriented language, therefore trying to do
purely Object-Oriented stuff is usually a mistake.







I think that what is going on here is this:

The call to for_each is inside a class, and originally rhs_callback was
outside the class and everything worked. Then for some unknown reason,
you decided to put rhs_callback inside the class. Doing that changed the
signature of the function, making it incompatible with the for_each
function. The boost::bind construct creates a function of the original
signature by attaching 'this' to the new, unneeded, parameter of the
rhs_callback function.

I'm guessing in the above with very little information to go on, but
there you go.








The above doesn't belong inside a class because it doesn't use the
'this' object. You could force it if you wanted by making it static in a
class, but then the only logical place to put it would be in the point
"class."

In other words:

struct point {
   double x;
   double y;
   static bool x_comparison(const point& lhs, const point& rhs);

};

Then you can:

pointKeeper.sort(&point::x_comparison);



Of course they won't compile. The sort function only passes in two
objects (two points.) ClassName::x_comparison requires three objects
(two points and one object of type ClassName.)

Great! I am moving forward thanks your suggestion, Daniel. My code
finally compiles.

The code now looks like this:

struct point {
double x;
double y;
static bool x_comparison(const point& pt1, const point& pt2)
{return pt1.x < pt2.x;};
static bool y_comparison(const point& pt1, const point& pt2)
{return pt1.y < pt2.y;};
};

Having executable code inside a .h files bothers me, however. In this
particular case the code is very succinct, but what if it were
massive? How can I place the executable back in the *.cpp file?

-Ramon

ps: BTW: Now both versions below compile, I guess I will find out at
run (link?) time which one is the correct one:

pointKeeper.sort(&point::x_comparison);
pointKeeper.sort(point::x_comparison);

 

[Ramon F Herrera]

Daniel T. ha scritto:




Or you just overload operator< for "point" and use the simple version of
sort() which doesn't take a predicate argument.

struct point {
  double x;
  double y;
  bool operator<(const point &rhs) const;

};

> pointKeeper.sort();

Hi Christian,

I actually need to sort my list by 'x' value, and later on, by 'y'
value. I only posted half the problem for simplicity's sake. Do yo
have a similar suggestion (i.e., with operator overloading)?

Thx,

-RFH

 

[Ramon F Herrera]

> Providing an operator< instead is less ugly though IMO
> (and it is not a "hack" but a normal C++ thing to do).

Couldn't agree more. I was just making levity of Christian's last
name.

I am not too clear on the use of 'lhs' and 'rhs', however. I need a
way to sort based on x and another to sort based on y.

TIA,

-RFH

 

[Lucius Sanctimonious]

(e-mail address removed):








> You are trying to make it hard for us, right!

Sorry!! I figured the formatting would look better (be more readable)
that way. Will copy & paste next...

-Ramon

 

[Ramon F Herrera]

(e-mail address removed):








> You are trying to make it hard for us, right!

Sorry! I figured that way it would be more readable.

There you go:

http://patriot.net/~ramon/minimal.h.txt
http://patriot.net/~ramon/minimal.cpp.txt

-Ramon

 

[Ulrich Eckhardt]

The code now looks like this:

struct point {
double x;
double y;
static bool x_comparison(const point& pt1, const point& pt2)
{return pt1.x < pt2.x;};
static bool y_comparison(const point& pt1, const point& pt2)
{return pt1.y < pt2.y;};
};

For the record, there are two semicolons after the body of the functions
that don't make sense. They are legal C++ though, IMHO due to an oversight
in the standard.

Having executable code inside a .h files bothers me, however.

Putting small functions implicitly (as here) or explicitly declared as
inline into headers is not that unusual.

In this particular case the code is very succinct, but what if it
were massive? How can I place the executable back in the *.cpp file?

You can declare it in the header and implement it in any other file, just
like any member function. Note that the separate implementation should leave
out the "static" though.

One note btw: Someone mentioned that these comparison functions should
belong into class point, but I disagree. If they really belong to that class
or if they complement it, I would consider putting them as class-static
functions or into the same namespace. Otherwise, if you only need those
functions in the context of different code, I would also consider putting
them in there, e.g. into an anonymous namespace of a translation unit.

ps: BTW: Now both versions below compile, I guess I will find out at
run (link?) time which one is the correct one:

pointKeeper.sort(&point::x_comparison);
pointKeeper.sort(point::x_comparison);

The first one is the only correct one, though the second one is also
sometimes accepted. Note that only plain functions implicitly convert to a
function pointer for reasons of C compatibility.


Good luck!

Uli