Should I use a conversion operator for the struct I'm wrapping up?

Question

I'm wrapping up a struct and functions related to that struct in a C++ class.

class Surface
{
public:
   operator SDL_Surface* () { return this->surf_; } // good idea?
private:
   SDL_Surface* surf_;      
}

Is using a conversion operator in this case a good idea? Or will I run into problems? Any alternatives?

I've done things like this before, and it has worked out ok, but you probably don't want to do it until you find a real use case. — Vaughn Cato, Jul 29 '12 at 16:22
An alternative is a member with a suitable name. `std::string::c_str` and `std::unique_ptr::get` are examples of such members. — Luc Danton, Jul 29 '12 at 16:29

score 4 · Accepted Answer · answered Jul 29 '12 at 16:22

4

It's a terribly bad idea. The SDL_Surface* should remain entirely encapsulated within the class, accessible only to other classes which are part of your SDL abstraction. In addition, you should be smart pointing to it to ensure it's safe destruction.

answered Jul 29 '12 at 16:22

Puppy

144,682
38
256
465

2

AMEN! (Is this a C++ answer or just religion?) – Emilio Garavaglia Jul 29 '12 at 16:30
@EmilioGaravaglia, I have seen enough DeadMG answers to know it's religious. – Shahbaz Jul 29 '12 at 16:31
@Shahbaz: and judging the up-votes ... many other people like to be, even before thinking about! – Emilio Garavaglia Jul 29 '12 at 16:46
1

There's nothing religious about it. Encapsulation is the core tenet of OOP and good programming, and the proposed operator clearly violates it massively. Secondly, smart pointing is simply good C++. – Puppy Jul 29 '12 at 16:59
@DeadMG: It IS religious: you talk about OOP as "the one and only programming techiniqe" even without the OP mentions it. Pragmatism suggests that if you encapsulate and then expose everything... just let the access available: you got the same result is far less of the time. see here: http://stackoverflow.com/a/11202368/924727 – Emilio Garavaglia Jul 30 '12 at 06:15

score 1 · Answer 2 · answered Jul 29 '12 at 16:19

1

I would suggest using std::shared_ptr<SDL_Surface> with SDL_FreeSurface as custom deleter.

answered Jul 29 '12 at 16:19

yuri kilochek

12,709
2
32
59

score 1 · Answer 3 · answered Jul 29 '12 at 19:41

Explicit is good, implicit is bad.

Example from the standard library: to convert a std::string to char const*, you have to call c_str(). This saves you from doing Unholy Things where you don't notice that a conversion is involved, and it prevents surprising results with function overloads.

As any rule of thumb this rule has exceptions. It should not be a substitute for thinking. But generally, very good reasons are required to make conversions implicit.

score 0 · Answer 4 · answered Jul 29 '12 at 16:44

It depends.

If your surface is intended as an extension of SDL_Surface, and must participate in all operation / functions SDL_Surface is involved into, you will probably like to have a way to expose the "basic" element.

But, instead of returning a pointer from a class (something that has a different indirection level in expressions), you would probably had better to return a reference or a cost-reference or both:

class Surface
{
public:
   operator SDL_Surface& () { return *surf_; }
   operator const SDL_Surface& () const { return *surf_; }
private:
   SDL_Surface* surf_;      
}

(Note: avoid that C#-ish Javanese this->xxxx style...)

If your Surface is just another thing, that needs an SDL_Surface to work with, bat has all another application domain, than no implicit conversions should be available.

NOTE: In the above note I'm not doing any assumption about Surface and SDL_Surface lifetime or ownership. I just assume both object exist whenever they have to participate in whatever operation. Who creates and destroys them an in which order and when, is all another story. Note that the usage of reference counting smart pointer don't solve the general problem: until you don't define an ownership direction, forgetting about pointers in favor of shared_ptr is just a good way to make memory leaks because of circular reference. A very very common mistake, especially from Java programmers.

Moral of the story: be sure of what you want to do with those objects, before decide how create and delete them, witch king of "pointer" to use to hold them and if enable or not implicit conversions and which.

Even better, instead of keeping a pointer, keep an object: `SDL_Surface surf_;` — Shahbaz, Jul 29 '12 at 17:15

Should I use a conversion operator for the struct I'm wrapping up?

4 Answers4