> Short form:
>
> Fl_Shared_Image::copy() should return the proper
> type (Fl_Shared_Image* instead of Fl_Image*), and
> Fl_Shared_Image::refcount() should be private or
> protected.
IMHO No, It should _NOT_ return anything else than an Fl_Image.
This is also the base of polymorphic behavior, this virtual method
should always keep the _same_ signature in all derived classes (so it should
always return the base class type, here Fl_Image).
This question has been raised in a former STR and has been replied similarly by
Mike and me.
AFAIR, Some compilers will even raise warnings/errors if you try to change the
signature on the fly of a virtual function ...
>
>
> Long form:
>
> Problem #1:
>
> There are two copy methods:
>
> virtual Fl_Image *copy(int W, int H);
> Fl_Image *copy() { return copy(w(), h()); }
>
> Both are declared to return Fl_Image*. In fact they _do_ return
> an Fl_Shared_Image* pointer, the cast is done implicitly:
>
> Full code from Fl_Shared_Image.cxx:
>
> Fl_Image *
> Fl_Shared_Image::copy(int W, int H) {
> Fl_Image *temp_image; // New image file
> Fl_Shared_Image *temp_shared; // New shared image
>
> // Make a copy of the image we're sharing...
> if (!image_) temp_image = 0;
> else temp_image = image_->copy(W, H);
>
> // Then make a new shared image...
> temp_shared = new Fl_Shared_Image();
>
> temp_shared->name_ = new char[strlen(name_) + 1];
> strcpy((char *)temp_shared->name_, name_);
>
> temp_shared->refcount_ = 1;
> temp_shared->image_ = temp_image;
> temp_shared->alloc_image_ = 1;
>
> temp_shared->update();
>
> return temp_shared;
> }
>
> IMHO this may be unintentional (cut'n'paste problem?), but it is
> documented this way. However, the user is required to release the
> shared image later, and thus s/he *needs* to cast the pointer back
> to an (Fl_Shared_Image*) !
>
> Here is some (correct!) example code, posted today by Ian (I hope
> you don't mind that I use it here - the texture example works
> like a charm ;-) - thanks also to Greg).
>
> Fl_Image *temp;
> if (img->w() > img->h())
> temp = img->copy(TEX_W, TEX_H * img->h() / img->w());
> else
> temp = img->copy(TEX_W * img->w() / img->h(), TEX_H);
>
> img->release();
> img = (Fl_Shared_Image *)temp;
>
> The cast in the last line wouldn't be necessary if
> Fl_Shared_Image::copy() would return what it should !
>
> Thus my question: Should this be changed? IMHO yes, but what
> about compatibility? I think that most code would compile and
> work okay, even if we changed it. And I tested it with Ian's
> example code after my local change: it compiled and linked
> without error, as I expected, but maybe there is other code?
>
> So here is my vote:
>
> +1 for making Fl_Shared_Image::copy() return Fl_Shared_Image*.
So -1 see upper for details.
>
>
> -----
>
> Problem #2:
>
> Fl_Shared_Image::refcount() should *REALLY* be private or at least
> protected! This would prevent code like this:
>
> while (img->refcount()>0) img->release();
>
> I found this code in Fl_Help_View.cxx :-(, and I'm looking for
> a better solution, but that's another story...
>
> For those who are not familiar with Fl_Shared_Image:
>
> If you call release() and the refcount goes down to 0, the
> Fl_Shared_Image is deleted, and thus the "img" pointer would
> point to a non-existent object, and then...
>
>
> My vote:
>
> +1 for making refcount() private.
-1 for making it private because derived classes may need special memory mgt.
+1 for making it protected, would have the desired effect that you want except
it will pernit more flexibility in derived classes.
>
> -----
>
> I'm aware that these changes would be API (and ABI) changes, and
> I'm not sure if we should do this now, but IMHO this should be
> done ASAP.
Agreed 100% for point 2.
>
> And, as noted recently by someone (I don't remember who): there are
> more methods in Fl_Image and friends that should be checked for
> proper attributes (public, protected, private). Wouldn't this be
> a good time to do it, before we release FLTK 1.3.0 ?
>
> Comments or suggestions, please ?
>
> Albrecht
Fabien
_______________________________________________
fltk-dev mailing list
[email protected]
http://lists.easysw.com/mailman/listinfo/fltk-dev
